commit 3c1b634086934ed630c7e4058a7559681010148e
parent 21bf65b55f3997385820ca4015eee74b607e06fa
Author: pstanciu <pstanciu@mozilla.com>
Date: Mon, 15 Dec 2025 01:30:03 +0200
Revert "Bug 2005618 - Pack subpixel positioned glyphs in to one atlas alloc r=gfx-reviewers,lsalzman" for causing wr failures @highlight-text-across-elements.html
This reverts commit ea7162586cdb9856bf687054064de252ad955730.
Diffstat:
10 files changed, 23 insertions(+), 209 deletions(-)
diff --git a/gfx/wr/webrender/res/ps_text_run.glsl b/gfx/wr/webrender/res/ps_text_run.glsl
@@ -101,11 +101,8 @@ void main() {
PictureTask task = fetch_picture_task(ph.picture_task_address);
int glyph_index = instance.segment_index;
- int color_mode = instance.flags & 0xF;
- int subpx_offset_x = (instance.flags >> 4) & 0x3;
- int subpx_offset_y = (instance.flags >> 6) & 0x3;
- int subpx_dir = (instance.flags >> 8) & 0x3;
- int is_packed_glyph = (instance.flags >> 10) & 0x1;
+ int subpx_dir = (instance.flags >> 8) & 0xff;
+ int color_mode = instance.flags & 0xff;
// Note that the reference frame relative offset is stored in the prim local
// rect size during batching, instead of the actual size of the primitive.
@@ -121,16 +118,6 @@ void main() {
GlyphResource res = fetch_glyph_resource(instance.resource_address);
- // For multi-variant glyphs, adjust the UV rect to select the correct quarter
- // of the packed texture based on subpixel offset.
- // This must happen before geometry calculations since the glyph rect size depends on the UV rect.
- if (is_packed_glyph != 0) {
- int variant_index = (subpx_dir == SUBPX_DIR_HORIZONTAL) ? subpx_offset_x : subpx_offset_y;
- float quarter_width = (res.uv_rect.z - res.uv_rect.x) * 0.25;
- res.uv_rect.x = res.uv_rect.x + float(variant_index) * quarter_width;
- res.uv_rect.z = res.uv_rect.x + quarter_width;
- }
-
vec2 snap_bias = get_snap_bias(subpx_dir);
// Glyph space refers to the pixel space used by glyph rasterization during frame
diff --git a/gfx/wr/webrender/src/batch.rs b/gfx/wr/webrender/src/batch.rs
@@ -2015,9 +2015,6 @@ impl BatchBuilder {
glyph.index_in_text_run,
glyph.uv_rect_address,
color_mode,
- glyph.subpx_offset_x,
- glyph.subpx_offset_y,
- glyph.is_packed_glyph,
));
}
},
diff --git a/gfx/wr/webrender/src/glyph_cache.rs b/gfx/wr/webrender/src/glyph_cache.rs
@@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use api::{FontKey, FontInstanceKey, IdNamespace};
-use glyph_rasterizer::{FontInstance, GlyphFormat, GlyphCacheKey, GlyphRasterizer};
+use glyph_rasterizer::{FontInstance, GlyphFormat, GlyphKey, GlyphRasterizer};
use crate::internal_types::{FrameId, FrameStamp, FastHashMap};
use crate::resource_cache::ResourceClassCache;
use std::sync::Arc;
@@ -16,7 +16,6 @@ use crate::texture_cache::TextureCacheHandle;
pub struct CachedGlyphInfo {
pub format: GlyphFormat,
pub texture_cache_handle: TextureCacheHandle,
- pub is_packed_glyph: bool,
}
#[cfg_attr(feature = "capture", derive(Serialize))]
@@ -63,7 +62,7 @@ pub struct GlyphKeyCacheInfo {
last_frame_used: FrameId,
}
-pub type GlyphKeyCache = ResourceClassCache<GlyphCacheKey, GlyphCacheEntry, GlyphKeyCacheInfo>;
+pub type GlyphKeyCache = ResourceClassCache<GlyphKey, GlyphCacheEntry, GlyphKeyCacheInfo>;
impl GlyphKeyCache {
pub fn eviction_notice(&self) -> &EvictionNotice {
@@ -74,7 +73,7 @@ impl GlyphKeyCache {
self.clear();
}
- pub fn add_glyph(&mut self, key: GlyphCacheKey, value: GlyphCacheEntry) {
+ pub fn add_glyph(&mut self, key: GlyphKey, value: GlyphCacheEntry) {
self.insert(key, value);
}
diff --git a/gfx/wr/webrender/src/gpu_types.rs b/gfx/wr/webrender/src/gpu_types.rs
@@ -561,25 +561,14 @@ impl GlyphInstance {
glyph_index_in_text_run: i32,
glyph_uv_rect: GpuBufferAddress,
color_mode: ShaderColorMode,
- subpx_offset_x: u8,
- subpx_offset_y: u8,
- is_packed_glyph: bool,
) -> PrimitiveInstanceData {
- // Pack subpixel offsets and multi-variant flag into upper 16 bits of data[2]
- // After instance.flags extraction (>> 16), shader sees:
- // bits 0-3: color_mode, bits 4-5: subpx_offset_x, bits 6-7: subpx_offset_y,
- // bits 8-9: subpx_dir, bit 10: is_packed_glyph
- let packed_flags = (((is_packed_glyph as u32) & 0x1) << 26)
- | (((subpx_dir as u32) & 0x3) << 24)
- | (((subpx_offset_y as u32) & 0x3) << 22)
- | (((subpx_offset_x as u32) & 0x3) << 20)
- | (((color_mode as u32) & 0xF) << 16);
-
PrimitiveInstanceData {
data: [
self.prim_header_index.0 as i32,
clip_task.0 as i32,
- packed_flags as i32 | glyph_index_in_text_run,
+ (subpx_dir as u32 as i32) << 24
+ | (color_mode as u32 as i32) << 16
+ | glyph_index_in_text_run,
glyph_uv_rect.as_int(),
],
}
diff --git a/gfx/wr/webrender/src/resource_cache.rs b/gfx/wr/webrender/src/resource_cache.rs
@@ -67,9 +67,6 @@ pub struct GlyphFetchResult {
pub offset: DevicePoint,
pub size: DeviceIntSize,
pub scale: f32,
- pub subpx_offset_x: u8,
- pub subpx_offset_y: u8,
- pub is_packed_glyph: bool,
}
// These coordinates are always in texels.
@@ -1286,8 +1283,7 @@ impl ResourceCache {
font,
glyph_keys,
|key| {
- let cache_key = key.cache_key();
- if let Some(entry) = glyph_key_cache.try_get(&cache_key) {
+ if let Some(entry) = glyph_key_cache.try_get(key) {
match entry {
GlyphCacheEntry::Cached(ref glyph) => {
if !texture_cache.request(&glyph.texture_cache_handle, gpu_buffer) {
@@ -1302,7 +1298,7 @@ impl ResourceCache {
}
};
- glyph_key_cache.add_glyph(cache_key, GlyphCacheEntry::Pending);
+ glyph_key_cache.add_glyph(*key, GlyphCacheEntry::Pending);
true
}
@@ -1336,10 +1332,9 @@ impl ResourceCache {
debug_assert!(fetch_buffer.is_empty());
for (loop_index, key) in glyph_keys.iter().enumerate() {
- let cache_key = key.cache_key();
- let (cache_item, glyph_format, is_packed_glyph) = match *glyph_key_cache.get(&cache_key) {
+ let (cache_item, glyph_format) = match *glyph_key_cache.get(key) {
GlyphCacheEntry::Cached(ref glyph) => {
- (self.texture_cache.get(&glyph.texture_cache_handle), glyph.format, glyph.is_packed_glyph)
+ (self.texture_cache.get(&glyph.texture_cache_handle), glyph.format)
}
GlyphCacheEntry::Blank | GlyphCacheEntry::Pending => continue,
};
@@ -1352,16 +1347,12 @@ impl ResourceCache {
current_texture_id = cache_item.texture_id;
current_glyph_format = glyph_format;
}
- let (subpx_offset_x, subpx_offset_y) = key.subpixel_offset();
fetch_buffer.push(GlyphFetchResult {
index_in_text_run: loop_index as i32,
uv_rect_address: gpu_buffer.resolve_handle(cache_item.uv_rect_handle),
offset: DevicePoint::new(cache_item.user_data[0], cache_item.user_data[1]),
size: cache_item.uv_rect.size(),
scale: cache_item.user_data[2],
- subpx_offset_x: subpx_offset_x as u8,
- subpx_offset_y: subpx_offset_y as u8,
- is_packed_glyph,
});
}
@@ -1513,7 +1504,6 @@ impl ResourceCache {
self.glyph_rasterizer.resolve_glyphs(
|job, can_use_r8_format| {
let GlyphRasterJob { font, key, result } = job;
- let cache_key = key.cache_key();
let glyph_key_cache = cached_glyphs.get_glyph_key_cache_for_font_mut(&*font);
let glyph_info = match result {
Err(_) => GlyphCacheEntry::Blank,
@@ -1546,11 +1536,10 @@ impl ResourceCache {
GlyphCacheEntry::Cached(CachedGlyphInfo {
texture_cache_handle,
format: glyph.format,
- is_packed_glyph: glyph.is_packed_glyph,
})
}
};
- glyph_key_cache.insert(cache_key, glyph_info);
+ glyph_key_cache.insert(key, glyph_info);
},
profile,
);
diff --git a/gfx/wr/wr_glyph_rasterizer/src/platform/macos/font.rs b/gfx/wr/wr_glyph_rasterizer/src/platform/macos/font.rs
@@ -787,7 +787,6 @@ impl FontContext {
GlyphType::Vector => font.get_glyph_format(),
},
bytes: rasterized_pixels,
- is_packed_glyph: false,
})})
}
}
diff --git a/gfx/wr/wr_glyph_rasterizer/src/platform/unix/font.rs b/gfx/wr/wr_glyph_rasterizer/src/platform/unix/font.rs
@@ -1123,7 +1123,6 @@ impl FontContext {
scale,
format: glyph_format,
bytes: final_buffer,
- is_packed_glyph: false,
})
}
}
diff --git a/gfx/wr/wr_glyph_rasterizer/src/platform/windows/font.rs b/gfx/wr/wr_glyph_rasterizer/src/platform/windows/font.rs
@@ -638,7 +638,6 @@ impl FontContext {
scale: (if bitmaps { y_scale.recip() } else { 1.0 }) as f32,
format,
bytes: bgra_pixels,
- is_packed_glyph: false,
})
}
}
diff --git a/gfx/wr/wr_glyph_rasterizer/src/rasterizer.rs b/gfx/wr/wr_glyph_rasterizer/src/rasterizer.rs
@@ -1098,27 +1098,11 @@ impl Into<f64> for SubpixelOffset {
}
}
-impl SubpixelOffset {
- fn to_f32(self) -> f32 {
- match self {
- SubpixelOffset::Zero => 0.0,
- SubpixelOffset::Quarter => 0.25,
- SubpixelOffset::Half => 0.5,
- SubpixelOffset::ThreeQuarters => 0.75,
- }
- }
-}
-
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug, Ord, PartialOrd)]
#[cfg_attr(feature = "capture", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct GlyphKey(u32);
-#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug, Ord, PartialOrd)]
-#[cfg_attr(feature = "capture", derive(Serialize))]
-#[cfg_attr(feature = "replay", derive(Deserialize))]
-pub struct GlyphCacheKey(u32);
-
impl GlyphKey {
pub fn new(
index: u32,
@@ -1132,49 +1116,22 @@ impl GlyphKey {
};
let sox = SubpixelOffset::quantize(dx);
let soy = SubpixelOffset::quantize(dy);
- assert_eq!(0, index & 0xFC000000);
+ assert_eq!(0, index & 0xF0000000);
- GlyphKey(index | (sox as u32) << 26 | (soy as u32) << 28 | (subpx_dir as u32) << 30)
+ GlyphKey(index | (sox as u32) << 28 | (soy as u32) << 30)
}
pub fn index(&self) -> GlyphIndex {
- self.0 & 0x03FFFFFF
+ self.0 & 0x0FFFFFFF
}
- pub fn subpixel_offset(&self) -> (SubpixelOffset, SubpixelOffset) {
- let x = (self.0 >> 26) as u8 & 3;
- let y = (self.0 >> 28) as u8 & 3;
+ fn subpixel_offset(&self) -> (SubpixelOffset, SubpixelOffset) {
+ let x = (self.0 >> 28) as u8 & 3;
+ let y = (self.0 >> 30) as u8 & 3;
unsafe {
(mem::transmute(x), mem::transmute(y))
}
}
-
- pub fn subpixel_dir(&self) -> SubpixelDirection {
- let dir = (self.0 >> 30) as u8 & 3;
- unsafe {
- mem::transmute(dir as u32)
- }
- }
-
- pub fn cache_key(&self) -> GlyphCacheKey {
- let index = self.index();
- let subpx_dir = self.subpixel_dir();
- assert_eq!(0, index & 0xFC000000);
- GlyphCacheKey(index | (subpx_dir as u32) << 30)
- }
-}
-
-impl GlyphCacheKey {
- pub fn index(&self) -> GlyphIndex {
- self.0 & 0x03FFFFFF
- }
-
- pub fn subpixel_dir(&self) -> SubpixelDirection {
- let dir = ((self.0 >> 30) & 3) as u32;
- unsafe {
- mem::transmute(dir)
- }
- }
}
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
@@ -1333,7 +1290,6 @@ pub struct RasterizedGlyph {
pub scale: f32,
pub format: GlyphFormat,
pub bytes: Vec<u8>,
- pub is_packed_glyph: bool,
}
impl RasterizedGlyph {
@@ -1711,69 +1667,6 @@ pub type GlyphRasterResult = Result<RasterizedGlyph, GlyphRasterError>;
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct GpuGlyphCacheKey(pub u32);
-fn pack_glyph_variants_horizontal(variants: &[RasterizedGlyph]) -> RasterizedGlyph {
- // Pack 4 glyph variants horizontally into a single texture.
- // Normalize both left and top offsets via padding so all variants can use the same base offsets.
-
- let min_left = variants.iter().map(|v| v.left.floor()).fold(f32::INFINITY, f32::min);
- let max_top = variants.iter().map(|v| v.top.floor()).fold(f32::NEG_INFINITY, f32::max);
-
- // Slot width must accommodate the widest variant plus left padding
- let slot_width = variants.iter()
- .map(|v| v.width + (v.left.floor() - min_left) as i32)
- .max().unwrap();
-
- // Slot height must accommodate the tallest variant plus top padding
- let slot_height = variants.iter()
- .map(|v| v.height + (max_top - v.top.floor()) as i32)
- .max().unwrap();
-
- let packed_width = slot_width * 4;
- let bpp = 4;
-
- let mut packed_bytes = vec![0u8; (packed_width * slot_height * bpp) as usize];
-
- for (variant_idx, variant) in variants.iter().enumerate() {
- // Compute padding needed to normalize both left and top offsets
- let left_pad = (variant.left.floor() - min_left) as i32;
- let top_pad = (max_top - variant.top.floor()) as i32;
- let slot_x = variant_idx as i32 * slot_width;
-
- for src_y in 0..variant.height {
- let dst_y = src_y + top_pad;
- if dst_y >= slot_height {
- break;
- }
-
- let dst_x = slot_x + left_pad;
- if dst_x < 0 {
- continue;
- }
-
- let src_row_start = (src_y * variant.width * bpp) as usize;
- let src_row_end = src_row_start + (variant.width * bpp) as usize;
- let dst_row_start = (dst_y * packed_width * bpp + dst_x * bpp) as usize;
- let dst_row_end = dst_row_start + (variant.width * bpp) as usize;
-
- if dst_row_end <= packed_bytes.len() && src_row_end <= variant.bytes.len() {
- packed_bytes[dst_row_start..dst_row_end]
- .copy_from_slice(&variant.bytes[src_row_start..src_row_end]);
- }
- }
- }
-
- RasterizedGlyph {
- top: max_top,
- left: min_left,
- width: packed_width,
- height: slot_height,
- scale: variants[0].scale,
- format: variants[0].format,
- bytes: packed_bytes,
- is_packed_glyph: true,
- }
-}
-
fn process_glyph(
context: &mut FontContext,
can_use_r8_format: bool,
@@ -1781,44 +1674,7 @@ fn process_glyph(
key: GlyphKey,
) -> GlyphRasterJob {
profile_scope!("glyph-raster");
-
- let subpx_dir = key.subpixel_dir();
-
- let result = if subpx_dir == SubpixelDirection::None {
- context.rasterize_glyph(&font, &key)
- } else {
- let offsets = [
- SubpixelOffset::Zero,
- SubpixelOffset::Quarter,
- SubpixelOffset::Half,
- SubpixelOffset::ThreeQuarters,
- ];
-
- let mut variants = Vec::with_capacity(4);
- for offset in &offsets {
- let variant_key = GlyphKey::new(
- key.index(),
- match subpx_dir {
- SubpixelDirection::Horizontal => DevicePoint::new(offset.to_f32(), 0.0),
- SubpixelDirection::Vertical => DevicePoint::new(0.0, offset.to_f32()),
- SubpixelDirection::None => DevicePoint::zero(),
- },
- subpx_dir,
- );
-
- match context.rasterize_glyph(&font, &variant_key) {
- Ok(glyph) => variants.push(glyph),
- Err(e) => return GlyphRasterJob {
- font: font,
- key: key.clone(),
- result: Err(e),
- },
- }
- }
-
- Ok(pack_glyph_variants_horizontal(&variants))
- };
-
+ let result = context.rasterize_glyph(&font, &key);
let mut job = GlyphRasterJob {
font: font,
key: key.clone(),
diff --git a/gfx/wr/wrench/reftests/text/reftest.list b/gfx/wr/wrench/reftests/text/reftest.list
@@ -19,8 +19,8 @@ skip_on(android,device) fuzzy(1,3692) fuzzy-if(platform(win),2,5585) fuzzy-if(pl
fuzzy(2,405) fuzzy-if(platform(swgl),2,1510) == split-batch.yaml split-batch-ref.yaml
# Next 3 tests affected by bug 1548099 on Android
skip_on(android) == shadow-red.yaml shadow-red-ref.yaml
-skip_on(android) fuzzy(1,999) fuzzy-if(platform(swgl),2,1331) == shadow-grey.yaml shadow-grey-ref.yaml
-skip_on(android) fuzzy(1,834) fuzzy-if(platform(swgl),2,1549) == shadow-grey-transparent.yaml shadow-grey-ref.yaml
+skip_on(android) fuzzy(1,999) fuzzy-if(platform(swgl),2,1324) == shadow-grey.yaml shadow-grey-ref.yaml
+skip_on(android) fuzzy(1,834) fuzzy-if(platform(swgl),2,1538) == shadow-grey-transparent.yaml shadow-grey-ref.yaml
== subtle-shadow.yaml subtle-shadow-ref.yaml
fuzzy(1,64) == shadow-atomic.yaml shadow-atomic-ref.yaml
fuzzy(1,64) == shadow-clip-rect.yaml shadow-atomic-ref.yaml
@@ -57,7 +57,7 @@ fuzzy(1,1) platform(linux) == two-shadows.yaml two-shadows.png
== shadow-fast-clip.yaml shadow-fast-clip-ref.yaml
skip_on(android,device) fuzzy-if(platform(win),1,18) == shadow-partial-glyph.yaml shadow-partial-glyph-ref.yaml # Fails on Pixel2
fuzzy(2,212) platform(linux) == shadow-transforms.yaml shadow-transforms.png
-fuzzy(2,372) platform(linux) == raster-space.yaml raster-space.png
+fuzzy(2,370) platform(linux) == raster-space.yaml raster-space.png
skip_on(android) skip_on(mac,>=10.14) != allow-subpixel.yaml allow-subpixel-ref.yaml # Android: we don't enable sub-px aa on this platform.
!= large-glyphs.yaml blank.yaml
!= large-line-decoration.yaml blank.yaml
