WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED WONTFIX
234290
Remove unnecessary TiledBacking virtual functions that aren't called by WebCore common layer
https://bugs.webkit.org/show_bug.cgi?id=234290
Summary
Remove unnecessary TiledBacking virtual functions that aren't called by WebCo...
Fujii Hironori
Reported
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
Attachments
Patch
(33.01 KB, patch)
2021-12-13 22:08 PST
,
Fujii Hironori
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Fujii Hironori
Comment 1
2021-12-13 22:08:31 PST
Created
attachment 447108
[details]
Patch
Simon Fraser (smfr)
Comment 2
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?
Fujii Hironori
Comment 3
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.
Fujii Hironori
Comment 4
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.
Simon Fraser (smfr)
Comment 5
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.
Fujii Hironori
Comment 6
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?
Fujii Hironori
Comment 7
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?
Fujii Hironori
Comment 8
2021-12-16 13:18:32 PST
Umm, this refactoring shouldn't be controversial. But, I give up this patch.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug