Bug 234290 - Remove unnecessary TiledBacking virtual functions that aren't called by WebCore common layer
Summary: Remove unnecessary TiledBacking virtual functions that aren't called by WebCo...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Compositing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on:
Blocks: 233676
  Show dependency treegraph
 
Reported: 2021-12-13 21:50 PST by Fujii Hironori
Modified: 2021-12-16 13:18 PST (History)
1 user (show)

See Also:


Attachments
Patch (33.01 KB, patch)
2021-12-13 22:08 PST, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2021-12-13 21:50:47 PST
Remove GraphicsLayerCA specific TiledBacking virtual functions

I'm going to implement TiledBacking subclass for WinCairo (Bug 233676).
TiledBacking has a lot of GraphicsLayerCA specific virtual functions.
Let's remove them.

Covariant return type will help to do that.

https://en.wikipedia.org/wiki/Covariant_return_type
https://en.cppreference.com/w/cpp/language/virtual
Comment 1 Fujii Hironori 2021-12-13 22:08:31 PST
Created attachment 447108 [details]
Patch
Comment 2 Simon Fraser (smfr) 2021-12-14 09:31:55 PST
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 3 Fujii Hironori 2021-12-14 12:01:50 PST
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 4 Fujii Hironori 2021-12-14 12:19:09 PST
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 5 Simon Fraser (smfr) 2021-12-14 23:10:38 PST
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 6 Fujii Hironori 2021-12-15 03:24:14 PST
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 7 Fujii Hironori 2021-12-15 23:05:26 PST
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?
Comment 8 Fujii Hironori 2021-12-16 13:18:32 PST
Umm, this refactoring shouldn't be controversial. But, I give up this patch.