Summary: | Remove unnecessary TiledBacking virtual functions that aren't called by WebCore common layer | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||
Component: | Compositing | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||
Status: | RESOLVED WONTFIX | ||||||
Severity: | Normal | CC: | simon.fraser | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 233676 | ||||||
Attachments: |
|
Description
Fujii Hironori
2021-12-13 21:50:47 PST
Created attachment 447108 [details]
Patch
Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review > Source/WebCore/ChangeLog:9 > + TiledBacking had a lot of GraphicsLayerCA specific virtual > + functions. It unnecessary made difficult to implement a subclass I'm not sure I agree with this. Doesn't any platform need the concepts of visible rect, coverage rect and might want to optimize live resize? Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review >> Source/WebCore/ChangeLog:9 >> + functions. It unnecessary made difficult to implement a subclass > > I'm not sure I agree with this. Doesn't any platform need the concepts of visible rect, coverage rect and might want to optimize live resize? This patch removes unnecessary virtual functions which are not called by WebCore common code. They are called only by GraphicsLayerCA implementation layer. If the WebCore common code will need the virtual methods, we can add them. Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review >>> Source/WebCore/ChangeLog:9 >>> + functions. It unnecessary made difficult to implement a subclass >> >> I'm not sure I agree with this. Doesn't any platform need the concepts of visible rect, coverage rect and might want to optimize live resize? > > This patch removes unnecessary virtual functions which are not called by WebCore common code. > They are called only by GraphicsLayerCA implementation layer. > If the WebCore common code will need the virtual methods, we can add them. I misunderstood the reason why we remove them. We should remove those TiledBacking virtual functions because they aren't called by WebCore layer, not because they are platform-specific. I should change the bug title and the change log. GraphicsContext interface has some platfrom specifix virtual functions. But, we can't remove them because they are called by WebCore layer, they are needed. Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review > Source/WebCore/platform/graphics/ca/PlatformCALayer.h:282 > + virtual TileController* tiledBacking() = 0; All the tiledBacking() functions that return a TileController need to be renamed. Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review >> Source/WebCore/platform/graphics/ca/PlatformCALayer.h:282 >> + virtual TileController* tiledBacking() = 0; > > All the tiledBacking() functions that return a TileController need to be renamed. How do you think about an idea renaming TileController to CATiledBacking or TiledBackingCA? Comment on attachment 447108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447108&action=review >>> Source/WebCore/platform/graphics/ca/PlatformCALayer.h:282 >>> + virtual TileController* tiledBacking() = 0; >> >> All the tiledBacking() functions that return a TileController need to be renamed. > > How do you think about an idea renaming TileController to CATiledBacking or TiledBackingCA? Because GraphicsLayerCA::tiledBacking() is a override function I can't rename even thouth this patch changed it to return TileController*. I think renaming TileController seems better. regardless of whether the method or the class will rename, I think I should separate patches for brevity. WDYT? Umm, this refactoring shouldn't be controversial. But, I give up this patch. |