Bug 1696039 - Align texture upload stride to multiples of 128 bytes on Adreno 3xx r=kvark

On Adreno 3xx we see intermittently corrupted or missing glyphs due to
texture uploads failing. This has only started occuring since glyphs
were switched to using R8 format textures, from RGBA8.

Due to performance reasons on other Adreno devices, we currently
ensure texture data has a stride aligned to 64 pixels. So the effect
of switching texture format is that the alignment switched from 256
bytes to 64 bytes.

It appears that PBO texture uploads must be performed with an
alignment of 128 bytes on Adreno 3xx, otherwise this corruption may
occur. Additionally, the 64 pixel requirement to hit the fast path
does not seem to apply to Adreno 3xx. Therefore this patch sets the
requirement to 128 bytes on Adreno 3xx, and leaves it as 64 pixels on
other Adreno devices.

It also renames optimal_pbo_stride to required_pbo_stride, to better
reflect the fact that this is now sometimes required for correctness
reasons, not just performance.

Differential Revision: https://phabricator.services.mozilla.com/D112058

gfx/wr/webrender/src/device/gl.rs

@@ -1121,7 +1121,10 @@ pub struct Device {
     /// format, we fall back to glTexImage*.
     texture_storage_usage: TexStorageUsage,
 
-    optimal_pbo_stride: StrideAlignment,
+    /// Required stride alignment for pixel transfers. This may be required for
+    /// correctness reasons due to driver bugs, or for performance reasons to
+    /// ensure we remain on the fast-path for transfers.
+    required_pbo_stride: StrideAlignment,
 
     /// Whether we must ensure the source strings passed to glShaderSource()
     /// are null-terminated, to work around driver bugs.
@@ -1646,12 +1649,19 @@ impl Device {
              //  (XXX: we apply this restriction to all GPUs to handle switching)
 
         let is_angle = renderer_name.starts_with("ANGLE");
+        let is_adreno_3xx = renderer_name.starts_with("Adreno (TM) 3");
 
-        // On certain GPUs PBO texture upload is only performed asynchronously
-        // if the stride of the data is a multiple of a certain value.
-        let optimal_pbo_stride = if is_adreno {
-            // On Adreno it must be a multiple of 64 pixels, meaning value in bytes
-            // varies with the texture format.
+        // Some GPUs require the stride of the data during texture uploads to be
+        // aligned to certain requirements, either for correctness or performance
+        // reasons.
+        let required_pbo_stride = if is_adreno_3xx {
+            // On Adreno 3xx, alignments of < 128 bytes can result in corrupted
+            // glyphs. See bug 1696039.
+            StrideAlignment::Bytes(NonZeroUsize::new(128).unwrap())
+        } else if is_adreno {
+            // On later Adreno devices it must be a multiple of 64 *pixels* to
+            // hit the fast path, meaning value in bytes varies with the texture
+            // format. This is purely an optimization.
             StrideAlignment::Pixels(NonZeroUsize::new(64).unwrap())
         } else if is_macos {
             // On AMD Mac, it must always be a multiple of 256 bytes.
@@ -1789,7 +1799,7 @@ impl Device {
             requires_null_terminated_shader_source,
             requires_unique_shader_source,
             requires_texture_external_unbind,
-            optimal_pbo_stride,
+            required_pbo_stride,
             dump_shader_source,
             surface_origin_is_top_left,
 
@@ -1860,8 +1870,8 @@ impl Device {
         return (self.max_depth_ids() - 1) as f32;
     }
 
-    pub fn optimal_pbo_stride(&self) -> StrideAlignment {
-        self.optimal_pbo_stride
+    pub fn required_pbo_stride(&self) -> StrideAlignment {
+        self.required_pbo_stride
     }
 
     pub fn upload_method(&self) -> &UploadMethod {
@@ -3068,7 +3078,7 @@ impl Device {
         let bytes_pp = format.bytes_per_pixel() as usize;
         let width_bytes = size.width as usize * bytes_pp;
 
-        let dst_stride = round_up_to_multiple(width_bytes, self.optimal_pbo_stride.num_bytes(format));
+        let dst_stride = round_up_to_multiple(width_bytes, self.required_pbo_stride.num_bytes(format));
 
         // The size of the chunk should only need to be (height - 1) * dst_stride + width_bytes,
         // however, the android emulator will error unless it is height * dst_stride.

gfx/wr/webrender/src/renderer/mod.rs

@@ -1412,8 +1412,8 @@ impl Renderer {
         self.device.preferred_color_formats().external
     }
 
-    pub fn optimal_texture_stride_alignment(&self, format: ImageFormat) -> usize {
-        self.device.optimal_pbo_stride().num_bytes(format).get()
+    pub fn required_texture_stride_alignment(&self, format: ImageFormat) -> usize {
+        self.device.required_pbo_stride().num_bytes(format).get()
     }
 
     pub fn set_clear_color(&mut self, color: Option<ColorF>) {

gfx/wr/webrender/src/screen_capture.rs

@@ -144,7 +144,7 @@ impl AsyncScreenshotGrabber {
         let read_size = match self.mode {
             AsyncScreenshotGrabberMode::ProfilerScreenshots => {
                 let stride = (screenshot_size.width * image_format.bytes_per_pixel()) as usize;
-                let rounded = round_up_to_multiple(stride, device.optimal_pbo_stride().num_bytes(image_format));
+                let rounded = round_up_to_multiple(stride, device.required_pbo_stride().num_bytes(image_format));
                 let optimal_width = rounded as i32 / image_format.bytes_per_pixel();
 
                 DeviceIntSize::new(