Bug 1564873 - Stop using mem::uninitialized to pass memory to the GPU. r=Gankro
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 18 Nov 2019 21:40:39 +0000
changeset 502483 582c1acfb3483e087316793a965497b9dfdbc208
parent 502482 7f64e4879257aefc6450321a66532576ca29079b
child 502484 635be62762b9f8fa94f3cb38e81d356a6b8f22bb
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGankro
bugs1564873
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1564873 - Stop using mem::uninitialized to pass memory to the GPU. r=Gankro Use Vec::reserve + as_ptr, then raw pointers. Alternative is to require T: Default, and then push T::default() or something. Differential Revision: https://phabricator.services.mozilla.com/D53360
gfx/wr/webrender/src/device/gl.rs
gfx/wr/webrender/src/renderer.rs
--- a/gfx/wr/webrender/src/device/gl.rs
+++ b/gfx/wr/webrender/src/device/gl.rs
@@ -3529,17 +3529,18 @@ impl<'a, T> Drop for TextureUploader<'a,
 
 impl<'a, T> TextureUploader<'a, T> {
     pub fn upload(
         &mut self,
         mut rect: DeviceIntRect,
         layer_index: i32,
         stride: Option<i32>,
         format_override: Option<ImageFormat>,
-        data: &[T],
+        data: *const T,
+        len: usize,
     ) -> usize {
         // Textures dimensions may have been clamped by the hardware. Crop the
         // upload region to match.
         let cropped = rect.intersection(
             &DeviceIntRect::new(DeviceIntPoint::zero(), self.target.texture.get_dimensions())
         );
         if cfg!(debug_assertions) && cropped.map_or(true, |r| r != rect) {
             warn!("Cropping texture upload {:?} to {:?}", rect, cropped);
@@ -3552,17 +3553,17 @@ impl<'a, T> TextureUploader<'a, T> {
         let bytes_pp = self.target.texture.format.bytes_per_pixel() as usize;
         let width_bytes = rect.size.width as usize * bytes_pp;
 
         let src_stride = stride.map_or(width_bytes, |stride| {
             assert!(stride >= 0);
             stride as usize
         });
         let src_size = (rect.size.height as usize - 1) * src_stride + width_bytes;
-        assert!(src_size <= data.len() * mem::size_of::<T>());
+        assert!(src_size <= len * mem::size_of::<T>());
 
         // for optimal PBO texture uploads the stride of the data in
         // the buffer may have to be a multiple of a certain value.
         let dst_stride = round_up_to_multiple(src_stride, self.target.optimal_pbo_stride);
         // The size of the PBO should only need to be (height - 1) * dst_stride + width_bytes,
         // however, the android emulator will error unless it is height * dst_stride.
         // See bug 1587047 for details.
         let dst_size = rect.size.height as usize * dst_stride;
@@ -3586,38 +3587,36 @@ impl<'a, T> TextureUploader<'a, T> {
                         buffer.usage,
                     );
                     buffer.size_allocated = dst_size;
                 }
 
                 if src_stride == dst_stride {
                     // the stride is already optimal, so simply copy
                     // the data as-is in to the buffer
-                    let elem_count = src_size / mem::size_of::<T>();
-                    assert_eq!(elem_count * mem::size_of::<T>(), src_size);
-                    let slice = &data[.. elem_count];
-
-                    gl::buffer_sub_data(
-                        self.target.gl,
+                    assert_eq!(src_size % mem::size_of::<T>(), 0);
+                    self.target.gl.buffer_sub_data_untyped(
                         gl::PIXEL_UNPACK_BUFFER,
-                        buffer.size_used as _,
-                        slice,
+                        buffer.size_used as isize,
+                        src_size as isize,
+                        data as *const _,
                     );
                 } else {
                     // copy the data line-by-line in to the buffer so
                     // that it has an optimal stride
                     let ptr = self.target.gl.map_buffer_range(
                         gl::PIXEL_UNPACK_BUFFER,
                         buffer.size_used as _,
                         dst_size as _,
-                        gl::MAP_WRITE_BIT | gl::MAP_INVALIDATE_RANGE_BIT);
+                        gl::MAP_WRITE_BIT | gl::MAP_INVALIDATE_RANGE_BIT,
+                    );
 
                     unsafe {
-                        let src: &[u8] = slice::from_raw_parts(data.as_ptr() as *const u8, src_size);
-                        let dst: &mut [u8] = slice::from_raw_parts_mut(ptr as *mut u8, dst_size);
+                        let src: &[mem::MaybeUninit<u8>] = slice::from_raw_parts(data as *const _, src_size);
+                        let dst: &mut [mem::MaybeUninit<u8>] = slice::from_raw_parts_mut(ptr as *mut _, dst_size);
 
                         for y in 0..rect.size.height as usize {
                             let src_start = y * src_stride;
                             let src_end = src_start + width_bytes;
                             let dst_start = y * dst_stride;
                             let dst_end = dst_start + width_bytes;
 
                             dst[dst_start..dst_end].copy_from_slice(&src[src_start..src_end])
@@ -3636,17 +3635,17 @@ impl<'a, T> TextureUploader<'a, T> {
                 });
                 buffer.size_used += dst_size;
             }
             None => {
                 self.target.update_impl(UploadChunk {
                     rect,
                     layer_index,
                     stride,
-                    offset: data.as_ptr() as _,
+                    offset: data as _,
                     format_override,
                 });
             }
         }
 
         dst_size
     }
 }
--- a/gfx/wr/webrender/src/renderer.rs
+++ b/gfx/wr/webrender/src/renderer.rs
@@ -1530,17 +1530,17 @@ impl GpuCacheTexture {
                         continue;
                     }
 
                     let rect = DeviceIntRect::new(
                         DeviceIntPoint::new(0, row_index as i32),
                         DeviceIntSize::new(MAX_VERTEX_TEXTURE_WIDTH as i32, 1),
                     );
 
-                    uploader.upload(rect, 0, None, None, &*row.cpu_blocks);
+                    uploader.upload(rect, 0, None, None, row.cpu_blocks.as_ptr(), row.cpu_blocks.len());
 
                     row.is_dirty = false;
                 }
 
                 rows_dirty
             }
             GpuCacheBus::Scatter { ref program, ref vao, count, .. } => {
                 device.disable_depth();
@@ -1590,35 +1590,39 @@ impl<T> VertexDataTexture<T> {
     fn size_in_bytes(&self) -> usize {
         self.texture.as_ref().map_or(0, |t| t.size_in_bytes())
     }
 
     fn update(&mut self, device: &mut Device, data: &mut Vec<T>) {
         debug_assert!(mem::size_of::<T>() % 16 == 0);
         let texels_per_item = mem::size_of::<T>() / 16;
         let items_per_row = MAX_VERTEX_TEXTURE_WIDTH / texels_per_item;
+        debug_assert_ne!(items_per_row, 0);
 
         // Ensure we always end up with a texture when leaving this method.
-        if data.is_empty() {
+        let mut len = data.len();
+        if len == 0 {
             if self.texture.is_some() {
                 return;
             }
-            data.push(unsafe { mem::uninitialized() });
-        }
-
-        // Extend the data array to be a multiple of the row size.
-        // This ensures memory safety when the array is passed to
-        // OpenGL to upload to the GPU.
-        if items_per_row != 0 {
-            while data.len() % items_per_row != 0 {
-                data.push(unsafe { mem::uninitialized() });
+            data.reserve(items_per_row);
+            len = items_per_row;
+        } else {
+            // Extend the data array to have enough capacity to upload at least
+            // a multiple of the row size.  This ensures memory safety when the
+            // array is passed to OpenGL to upload to the GPU.
+            let extra = len % items_per_row;
+            if extra != 0 {
+                let padding = items_per_row - extra;
+                data.reserve(padding);
+                len += padding;
             }
         }
 
-        let needed_height = (data.len() / items_per_row) as i32;
+        let needed_height = (len / items_per_row) as i32;
         let existing_height = self.texture.as_ref().map_or(0, |t| t.get_dimensions().height);
 
         // Create a new texture if needed.
         //
         // These textures are generally very small, which is why we don't bother
         // with incremental updates and just re-upload every frame. For most pages
         // they're one row each, and on stress tests like css-francine they end up
         // in the 6-14 range. So we size the texture tightly to what we need (usually
@@ -1652,19 +1656,21 @@ impl<T> VertexDataTexture<T> {
         // [1] https://software.intel.com/en-us/articles/opengl-performance-tips-power-of-two-textures-have-better-performance
         let logical_width =
             (MAX_VERTEX_TEXTURE_WIDTH - (MAX_VERTEX_TEXTURE_WIDTH % texels_per_item)) as i32;
 
         let rect = DeviceIntRect::new(
             DeviceIntPoint::zero(),
             DeviceIntSize::new(logical_width, needed_height),
         );
+
+        debug_assert!(len <= data.capacity(), "CPU copy will read out of bounds");
         device
             .upload_texture(self.texture(), &self.pbo, 0)
-            .upload(rect, 0, None, None, data);
+            .upload(rect, 0, None, None, data.as_ptr(), len);
     }
 
     fn deinit(mut self, device: &mut Device) {
         device.delete_pbo(self.pbo);
         if let Some(t) = self.texture.take() {
             device.delete_texture(t);
         }
     }
@@ -3450,22 +3456,24 @@ impl Renderer {
 
                     let bytes_uploaded = match source {
                         TextureUpdateSource::Bytes { data } => {
                             let mut uploader = self.device.upload_texture(
                                 texture,
                                 &self.texture_cache_upload_pbo,
                                 0,
                             );
+                            let data = &data[offset as usize ..];
                             uploader.upload(
                                 rect,
                                 layer_index,
                                 stride,
                                 format_override,
-                                &data[offset as usize ..],
+                                data.as_ptr(),
+                                data.len(),
                             )
                         }
                         TextureUpdateSource::External { id, channel_index } => {
                             let mut uploader = self.device.upload_texture(
                                 texture,
                                 &self.texture_cache_upload_pbo,
                                 0,
                             );
@@ -3492,17 +3500,18 @@ impl Renderer {
                                     panic!("Unexpected external texture {:?} for the texture cache update of {:?}", eid, id);
                                 }
                             };
                             let size = uploader.upload(
                                 rect,
                                 layer_index,
                                 stride,
                                 format_override,
-                                data,
+                                data.as_ptr(),
+                                data.len()
                             );
                             handler.unlock(id, channel_index);
                             size
                         }
                         TextureUpdateSource::DebugClear => {
                             let draw_target = DrawTarget::from_texture(
                                 texture,
                                 layer_index as usize,