(WIP) maintain ColorFilter construction details for embedding in DisplayList #31230
Conversation
flar
changed the title
flar
requested review from
chinmaygarde and
gw280
and removed request for
chinmaygarde
| if (!filter) { | ||
| current_color_filter_type_ = kNone; | ||
| Push<ClearColorFilterOp>(0, 0); | ||
| } else { |
There was a problem hiding this comment.
Would it be better to factor all of this logic out into our own ColorFilter class? We could have a ctor that takes an SkColorFilter and returns a ColorFilter that has this logic in it, and other ctors like ColorFilter(color, blendmode), ColorFilter(matrix)... etc. We would then store that object on the DisplayListBuilder and reference that in UpdateCurrentOpacityCompatibility().
I think that adding excessive logic like this to DisplayListBuilder makes the class more and more difficult to keep track of what's going on, especially if we are planning to start adding logic similar to this for other objects like ImageFilters and Shaders. Conceptually I see DisplayListBuilder as just a simple recorder/dispatcher class, but maybe that's a faulty view on my part.
There was a problem hiding this comment.
I have a local branch that I haven't turned into a PR yet where I do this. I create "display_list_color_filter.h/cc". There is a subclass that holds an SkColorFilter, but I don't have it do that logic in its constructor (because it's a constructor). I may make a factory static method, though, that encapsulates this logic.
flar
changed the title
2 participants
An initial stab at maintaining data for ColorFilter objects in the engine's own data structures so that we can use it directly for optimizations. This code may serve as a prototype for how we keep our own data around for other structures like ImageFilter and Shaders.
Note that even if we deal with a part of the system that can only communicate to us via SkCanvas operations, we can still dig out the Blend and Matrix types of Color Filters when they get used in a DisplayList. Only the Srgb <-> Linear ColorFilters can get lost in that type of flow.