Refactor ThebesLayerBuffer

NEW
Unassigned

Status

()

Core
Graphics: Layers
5 years ago
4 years ago

People

(Reporter: nrc, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [leave-open])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
ThebesLayerBuffer is an absolute mess. It handles buffer rotation and non-rotated buffers, Azure or Thebes drawing, component alpha and regular buffers; it interacts with single and double buffered content clients in odd ways; optionally works with texture clients (I guess that is only for OMTC, but seems to be not always). Many of these paths do not work with each and there is little to ensure internal consistency. For example, using Azure with a single buffered content client crashes. Azure and component alpha don't work together. There are overloaded methods and weird implicit dependencies. BeginPaint is one of the hairiest methods I've ever seen, and there is some pretty stiff competition around.

I believe buffer rotation might be going away at some point? Presumably once we have tiled thebes layers everywhere. That might take a while. It is probably worth deciding whether we should refactor ThebeLayerBuffer before or after that. But we definitely should improve it.

In the mean time I'm going to insert as many assertions as I can.
(Reporter)

Comment 1

5 years ago
Created attachment 778127 [details] [diff] [review]
tlb_asserts.patch

I'm not going to start refactoring, but here are some asserts to make it a little tiny bit more obvious when we go wrong sometimes.
Attachment #778127 - Flags: review?(matt.woodrow)
Attachment #778127 - Flags: review?(matt.woodrow) → review+
Thanks for the asserts, that will help.  Long term, we're getting rid of Thebes, would this refactoring be a first step to make that easier, or can we skip this bug if Thebes disappears?
I think we'll still need this class, or at least something that provides the equivalent functionality. A new name at least would be good.

We currently have lots of branches for azure vs thebes. All of those can go away once we have azure everywhere.
(Reporter)

Comment 4

5 years ago
(In reply to Milan Sreckovic [:milan] from comment #2)
> Thanks for the asserts, that will help.  Long term, we're getting rid of
> Thebes, would this refactoring be a first step to make that easier, or can
> we skip this bug if Thebes disappears?

Yeah, what Matt said. This is another class called ThebesSomething even though it is not part of Thebes and doesn't even rely on Thebes specifically.
(Reporter)

Comment 5

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3c3d36fa76e
Whiteboard: [leave-open]
(Reporter)

Updated

5 years ago
Attachment #778127 - Flags: checkin+

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c3c3d36fa76e
(Reporter)

Updated

4 years ago
Version: 21 Branch → Trunk
You need to log in before you can comment on or make changes to this bug.