New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dependencies: Upgrade to WebRender 0.64 #31486
Conversation
🔨 Triggering try run (#8138599281) for Linux WPT |
cc83708
to
48637aa
Compare
🔨 Triggering try run (#8138874362) for Linux WPT |
|
48637aa
to
46adfc1
Compare
🔨 Triggering try run (#8152820496) for Linux WPT |
|
46adfc1
to
e9c6183
Compare
🔨 Triggering try run (#8153363096) for Linux WPT |
|
e9c6183
to
8f0e41f
Compare
🔨 Triggering try run (#8161418063) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8161418063): Flaky unexpected result (19)
Stable unexpected results that are known to be intermittent (36)
Stable unexpected results (118)
|
|
6e5dbc7
to
6609165
Compare
🔨 Triggering try run (#8170673225) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8170673225): Flaky unexpected result (19)
Stable unexpected results that are known to be intermittent (34)
Stable unexpected results (98)
|
|
Test results for linux-wpt-layout-2020 from try job (#8249983144): Flaky unexpected result (23)
Stable unexpected results that are known to be intermittent (15)
Stable unexpected results (1)
|
|
7737a86
to
f8b12a5
Compare
🔨 Triggering try run (#8253266798) for Linux WPT |
Test results for linux-wpt-layout-2020 from try job (#8253266798): Flaky unexpected result (16)
Stable unexpected results that are known to be intermittent (18)
|
✨ Try run (#8253266798) succeeded. |
f8b12a5
to
c713698
Compare
node.content_rect, | ||
item_rect, | ||
LayoutVector2D::zero(), /* external_scroll_offset */ | ||
0, /* scroll_offst_generation */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0, /* scroll_offst_generation */ | |
0, /* scroll_offset_generation */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
components/servo/lib.rs
Outdated
@@ -336,7 +335,7 @@ where | |||
enable_subpixel_aa: pref!(gfx.subpixel_text_antialiasing.enabled) && | |||
!opts.debug.disable_subpixel_text_antialiasing, | |||
allow_texture_swizzling: pref!(gfx.texture_swizzling.enabled), | |||
clear_color: None, | |||
clear_color: ColorF::new(1.0, 1.0, 1.0, 1.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to set this to the value of the shell.background_color.rgba
pref? The compositor uses it in clear_background
but is it useful to make them both use the same value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I'm reading this value from the preferences now.
components/shared/compositing/lib.rs
Outdated
@@ -92,7 +92,7 @@ pub enum CompositorMsg { | |||
// sends a reply on the IpcSender, the constellation knows it's safe to | |||
// tear down the other threads associated with this pipeline. | |||
PipelineExited(PipelineId, IpcSender<()>), | |||
/// Runs a closure in the compositor thread. | |||
/// Runs a closure in, bool /* scrolled */ the compositor thread. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. The IDE did this somehow.
txn.set_scroll_offsets( | ||
external_scroll_id, | ||
vec![SampledScrollOffset { | ||
offset, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the offset
here intentionally not inverted unlike in the call above to set_scroll_offsets_for_node_with_external_scroll_id
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scroll tree and WebRender have opposite interpretations of the scroll offset unfortunately. I'd like to fix that in a followup, because it is pretty confusing indeed.
components/compositing/compositor.rs
Outdated
Ok(display_list_data) => display_list_data, | ||
Err(error) => { | ||
return warn!( | ||
"Could not recieve WebRender display list items data: {error}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Could not recieve WebRender display list items data: {error}" | |
"Could not receive WebRender display list items data: {error}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
components/compositing/compositor.rs
Outdated
Ok(display_list_data) => display_list_data, | ||
Err(error) => { | ||
return warn!( | ||
"Could not recieve WebRender display list cache data: {error}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
components/compositing/compositor.rs
Outdated
_ => return warn!("Could not recieve WebRender display list."), | ||
Err(error) => { | ||
return warn!( | ||
"Could not recieve WebRender display list spatial tree: {error}." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
components/compositing/compositor.rs
Outdated
let root_content_pipeline = match self.root_content_pipeline.id { | ||
Some(id) => id.to_webrender(), | ||
None => return, | ||
}; | ||
|
||
let zoom_factor = self.pinch_zoom_level(); | ||
let zoom_factor = self.device_pixels_per_page_pixel().0; | ||
if zoom_factor == 1.0 { | ||
transaction.set_root_pipeline(root_content_pipeline); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this path ever be taken on HiDPI devices, given that zoom_factor now includes the HiDPI factor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path will never be taken when there is a scaling factor. I've actually gone ahead and removed this. We will no longer use this path once the multiple webview change lands, so we might as well remove it now.
@@ -1465,15 +1489,17 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> { | |||
/// scrolling to the applicable scroll node under that point. If a scroll was |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub is not allowing me to add the comment on the exact line, but the documentation above this line needs to be updated to use DevicePoint
instead of WorldPoint
.
/// Perform a hit test at the given [`WorldPoint`]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fixed!
c713698
to
75aca3d
Compare
@mukilan Thanks for the review! I think I have responded to all comments. |
75aca3d
to
8e56739
Compare
bd12d21
to
b408d83
Compare
This brings the version of WebRender used in Servo up-to-date with Gecko upstream. The big change here is that HiDPI is no longer handled via WebRender. Instead this happens via a scale applied to the root layer in the compositor. In addition to this change, various changes are made to Servo to adapt to the new WebRender API. Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
b408d83
to
351bcfb
Compare
This brings the version of WebRender used in Servo up-to-date with Gecko
upstream. The big change here is that HiDPI is no longer handled via
WebRender. Instead this happens via a scale applied to the root layer in
the compositor. In addition to this change, various changes are made to
Servo to adapt to the new WebRender API.
Along with a wide variety of new test passes there are also some new failures:
contents are clipped before they are filtered by stacking
contexts. In reality the contents of stacking contexts should only
be clipped by clips from inside the stacking context and then the
stacking context should clip everythig post-filter.
stacking context contents
/css/filter-effects/drop-shadow-clipped-001.html
/css/filter-effects/will-change-blur-filter-under-clip.html
tests to fail, likely because the sticky nodes are only being being
updated due to sticky offsets on subsequent frames. I'm not sure
why this is happening exactly, but it's only an issues in tests.
/css/css-position/sticky/position-sticky-escape-scroller-004.html
/css/css-position/sticky/position-sticky-scroll-with-clip-and-abspos.html
/_mozilla/css/pixel_snapping_position_a.html
/css/css-transforms/transform-table-009.html
/css/css-transforms/transform-table-011.html
/css/css-transforms/transform-table-010.html
Co-authored-by: Mukilan Thiyagarajan mukilan@igalia.com
./mach build -d
does not report any errors./mach test-tidy
does not report any errors