Closed Bug 1409856 Opened 3 years ago Closed 3 years ago

Clean up coordinate system units in WR-related code

Categories

(Core :: Graphics: WebRender, enhancement, P1)

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: kats, Assigned: kats)

Details

(Whiteboard: [wr-mvp] [gfx-noted])

Attachments

(1 file)

Now that we've ditched layers-full WR, pretty much everything should be in LayoutDevice coordinate space. We should clean up the APIs and helpers to ensure this.
Is there any background I could read on this?
I wrote a blog post a while back that might help: https://staktrace.com/spout/entry.php?id=800

Also, comments in layout/base/Units.h
Right so according to that post, LayoutDevice != Layer, but a bunch of the diff you're testing is just replacing one with the other? How does that work?
Mostly because we did it wrong before :) Pretty much all of the values we are computing are actually in LayoutDevice space because we compute them from various layout properties, we just mislabeled them. Back when we had layers-full code we actually had things in layers space and so we made the APIs like StackingContextHelper::ToRelativeLayoutRect and wr::ToLayoutPoint take both. But then it looks like a bunch of layers conversions got cargo culted around into layout code which should have just kept things in LD coords. This undoes the mislabeling.
Ah perfect, that's exactly what I was hoping to hear!

I imagine we never noticed anything since the difference is (still?) only visible on mobile.
(In reply to Alexis Beingessner [:Gankro] from comment #7)
> I imagine we never noticed anything since the difference is (still?) only
> visible on mobile.

Yeah. And in most cases we are actually providing the right values. There's just a couple of places in the APZ code where we might not be, and so I left those as having some ToUnknown()/FromUnknown() stuff.
Comment on attachment 8919945 [details]
Bug 1409856 - Update all the WebRender code to use LayoutDevice units instead of Layer units.

https://reviewboard.mozilla.org/r/190892/#review196092
Attachment #8919945 - Flags: review?(jmuizelaar) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6600dc827ccf
Update all the WebRender code to use LayoutDevice units instead of Layer units. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/6600dc827ccf
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Priority: P3 → P1
Whiteboard: [gfx-noted][wr-mvp][triage] → [wr-mvp] [gfx-noted]
You need to log in before you can comment on or make changes to this bug.