mirror of
https://github.com/servo/servo.git
synced 2025-07-22 14:53:49 +01:00
Auto merge of #12742 - mrobinson:off-by-one-ng-2, r=pcwalton
Simplify and extend trans-stacking-context subpixel accumulation <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #10881 (github issue number if applicable). <!-- Either: --> - [x] There are tests for these changes OR - [x] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Simplify the situations in which subpixels are accumulated, so that it is only done for translation or identity transformation matrices. Also, apply accumulated subpixels to more operations in PaintContext. This fixes several pre-existing reftests and hopefully will eliminate off-by-one errors in flaky reftests. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12742) <!-- Reviewable:end -->
This commit is contained in:
bors-servo
GitHub
commit
3ae94fca70
5 changed files with 39 additions and 34 deletions
|
|
@ -18,7 +18,7 @@ use app_units::Au;
|
|||
use azure::azure::AzFloat;
|
||||
use azure::azure_hl::Color;
|
||||
use euclid::approxeq::ApproxEq;
|
||||
use euclid::num::Zero;
|
||||
use euclid::num::{One, Zero};
|
||||
use euclid::rect::TypedRect;
|
||||
use euclid::side_offsets::SideOffsets2D;
|
||||
use euclid::{Matrix2D, Matrix4D, Point2D, Rect, Size2D};
|
||||
|
|
@ -457,20 +457,18 @@ impl DisplayList {
|
|||
let transform = transform.translate(pixel_snapped_origin.x as AzFloat,
|
||||
pixel_snapped_origin.y as AzFloat,
|
||||
0.0).mul(&stacking_context.transform);
|
||||
let inverse_transform = transform.invert();
|
||||
|
||||
// Here we are trying to accumulate any subpixel distances across transformed
|
||||
// stacking contexts. This allows us transform stacking context with a
|
||||
// pixel-snapped transform, but continue to propagate any subpixels from stacking
|
||||
// context origins to children.
|
||||
let subpixel_offset = Point2D::new(origin.x.to_f32_px() - pixel_snapped_origin.x,
|
||||
origin.y.to_f32_px() - pixel_snapped_origin.y);
|
||||
let subpixel_offset = inverse_transform.transform_point(&subpixel_offset) -
|
||||
inverse_transform.transform_point(&Point2D::zero());;
|
||||
let subpixel_offset = Point2D::new(Au::from_f32_px(subpixel_offset.x),
|
||||
Au::from_f32_px(subpixel_offset.y));
|
||||
|
||||
(transform, subpixel_offset)
|
||||
if transform.is_identity_or_simple_translation() {
|
||||
let pixel_snapped_origin = Point2D::new(Au::from_f32_px(pixel_snapped_origin.x),
|
||||
Au::from_f32_px(pixel_snapped_origin.y));
|
||||
(transform, origin - pixel_snapped_origin)
|
||||
} else {
|
||||
// In the case of a more complicated transformation, don't attempt to
|
||||
// preserve subpixel offsets. This causes problems with reference tests
|
||||
// that do scaling and rotation and it's unclear if we even want to be doing
|
||||
// this.
|
||||
(transform, Point2D::zero())
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -1462,3 +1460,18 @@ impl WebRenderImageInfo {
|
|||
/// The type of the scroll offset list. This is only populated if WebRender is in use.
|
||||
pub type ScrollOffsetMap = HashMap<StackingContextId, Point2D<f32>>;
|
||||
|
||||
|
||||
pub trait SimpleMatrixDetection {
|
||||
fn is_identity_or_simple_translation(&self) -> bool;
|
||||
}
|
||||
|
||||
impl SimpleMatrixDetection for Matrix4D<f32> {
|
||||
#[inline]
|
||||
fn is_identity_or_simple_translation(&self) -> bool {
|
||||
let (_0, _1) = (Zero::zero(), One::one());
|
||||
self.m11 == _1 && self.m12 == _0 && self.m13 == _0 && self.m14 == _0 &&
|
||||
self.m21 == _0 && self.m22 == _1 && self.m23 == _0 && self.m24 == _0 &&
|
||||
self.m31 == _0 && self.m32 == _0 && self.m33 == _1 && self.m34 == _0 &&
|
||||
self.m44 == _1
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -122,6 +122,10 @@ struct CornerOrigin {
|
|||
}
|
||||
|
||||
impl<'a> PaintContext<'a> {
|
||||
pub fn to_nearest_azure_rect(&self, rect: &Rect<Au>) -> Rect<AzFloat> {
|
||||
rect.translate(&self.subpixel_offset).to_nearest_azure_rect(self.screen_pixels_per_px())
|
||||
}
|
||||
|
||||
pub fn screen_pixels_per_px(&self) -> ScaleFactor<PagePx, ScreenPx, f32> {
|
||||
self.screen_rect.as_f32().size.width / self.page_rect.size.width
|
||||
}
|
||||
|
|
@ -132,7 +136,7 @@ impl<'a> PaintContext<'a> {
|
|||
|
||||
pub fn draw_solid_color(&self, bounds: &Rect<Au>, color: Color) {
|
||||
self.draw_target.make_current();
|
||||
self.draw_target.fill_rect(&bounds.to_nearest_azure_rect(self.screen_pixels_per_px()),
|
||||
self.draw_target.fill_rect(&self.to_nearest_azure_rect(&bounds),
|
||||
PatternRef::Color(&ColorPattern::new(color)),
|
||||
None);
|
||||
}
|
||||
|
|
@ -160,7 +164,7 @@ impl<'a> PaintContext<'a> {
|
|||
}
|
||||
|
||||
pub fn draw_push_clip(&self, bounds: &Rect<Au>) {
|
||||
let rect = bounds.to_nearest_azure_rect(self.screen_pixels_per_px());
|
||||
let rect = self.to_nearest_azure_rect(bounds);
|
||||
let path_builder = self.draw_target.create_path_builder();
|
||||
|
||||
let left_top = Point2D::new(rect.origin.x, rect.origin.y);
|
||||
|
|
@ -211,7 +215,7 @@ impl<'a> PaintContext<'a> {
|
|||
let source_rect = Rect::new(Point2D::new(0.0, 0.0),
|
||||
Size2D::new(image_info.width as AzFloat,
|
||||
image_info.height as AzFloat));
|
||||
let dest_rect = bounds.to_nearest_azure_rect(scale);
|
||||
let dest_rect = self.to_nearest_azure_rect(bounds);
|
||||
|
||||
// TODO(pcwalton): According to CSS-IMAGES-3 § 5.3, nearest-neighbor interpolation is a
|
||||
// conforming implementation of `crisp-edges`, but it is not the best we could do.
|
||||
|
|
@ -1127,7 +1131,7 @@ impl<'a> PaintContext<'a> {
|
|||
radius: &BorderRadii<AzFloat>,
|
||||
color: Color,
|
||||
dash_size: DashSize) {
|
||||
let rect = bounds.to_nearest_azure_rect(self.screen_pixels_per_px());
|
||||
let rect = self.to_nearest_azure_rect(bounds);
|
||||
let draw_opts = DrawOptions::new(1.0, CompositionOp::Over, AntialiasMode::None);
|
||||
let border_width = match direction {
|
||||
Direction::Top => border.top,
|
||||
|
|
@ -1195,7 +1199,7 @@ impl<'a> PaintContext<'a> {
|
|||
border: &SideOffsets2D<f32>,
|
||||
radius: &BorderRadii<AzFloat>,
|
||||
color: Color) {
|
||||
let rect = bounds.to_nearest_azure_rect(self.screen_pixels_per_px());
|
||||
let rect = self.to_nearest_azure_rect(bounds);
|
||||
self.draw_border_path(&rect, direction, border, radius, color);
|
||||
}
|
||||
|
||||
|
|
@ -1203,7 +1207,7 @@ impl<'a> PaintContext<'a> {
|
|||
bounds: &Rect<Au>,
|
||||
border: &SideOffsets2D<f32>,
|
||||
shrink_factor: f32) -> Rect<f32> {
|
||||
let rect = bounds.to_nearest_azure_rect(self.screen_pixels_per_px());
|
||||
let rect = self.to_nearest_azure_rect(bounds);
|
||||
let scaled_border = SideOffsets2D::new(shrink_factor * border.top,
|
||||
shrink_factor * border.right,
|
||||
shrink_factor * border.bottom,
|
||||
|
|
@ -1406,7 +1410,7 @@ impl<'a> PaintContext<'a> {
|
|||
&end_point.to_nearest_azure_point(scale),
|
||||
stops,
|
||||
&Matrix2D::identity());
|
||||
self.draw_target.fill_rect(&bounds.to_nearest_azure_rect(scale),
|
||||
self.draw_target.fill_rect(&self.to_nearest_azure_rect(&bounds),
|
||||
PatternRef::LinearGradient(&pattern),
|
||||
None);
|
||||
}
|
||||
|
|
@ -1632,7 +1636,7 @@ impl<'a> PaintContext<'a> {
|
|||
self.draw_push_clip(&clip_region.main);
|
||||
for complex_region in &clip_region.complex {
|
||||
// FIXME(pcwalton): Actually draw a rounded rect.
|
||||
self.push_rounded_rect_clip(&complex_region.rect.to_nearest_azure_rect(scale),
|
||||
self.push_rounded_rect_clip(&self.to_nearest_azure_rect(&complex_region.rect),
|
||||
&complex_region.radii.to_radii_pixels(scale))
|
||||
}
|
||||
self.transient_clip = Some(clip_region)
|
||||
|
|
|
|||
|
|
@ -1,4 +0,0 @@
|
|||
[transform-input-017.htm]
|
||||
type: reftest
|
||||
expected: FAIL
|
||||
bug: https://github.com/servo/servo/issues/10881
|
||||
|
|
@ -1,4 +0,0 @@
|
|||
[transform-input-018.htm]
|
||||
type: reftest
|
||||
expected: FAIL
|
||||
bug: https://github.com/servo/servo/issues/10881
|
||||
|
|
@ -1,4 +0,0 @@
|
|||
[transform-input-019.htm]
|
||||
type: reftest
|
||||
expected:
|
||||
if os == "linux": FAIL
|
||||
Loading…
Add table
Add a link
Reference in a new issue