Bug 237024 - [LBSE] Begin layer-aware RenderSVGTransformableContainer implementation
Summary: [LBSE] Begin layer-aware RenderSVGTransformableContainer implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords: InRadar
Depends on: 237023
Blocks: 90738
  Show dependency treegraph
 
Reported: 2022-02-22 01:02 PST by Nikolas Zimmermann
Modified: 2022-03-07 03:16 PST (History)
15 users (show)

See Also:


Attachments
Patch (18.86 KB, patch)
2022-02-22 11:44 PST, Nikolas Zimmermann
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch, v2 (19.01 KB, patch)
2022-02-22 12:24 PST, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch, v3 (19.11 KB, patch)
2022-03-04 04:26 PST, Nikolas Zimmermann
rbuis: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 2022-02-22 01:02:36 PST
After renaming RenderSVGTransformableContainer -> LegacyRenderSVGTransformableContainer (bug 237023), RenderSVGTransformableContainer can be re-introduced for LBSE.
Comment 1 Nikolas Zimmermann 2022-02-22 11:44:43 PST
Created attachment 452887 [details]
Patch
Comment 2 Nikolas Zimmermann 2022-02-22 12:24:39 PST
Created attachment 452893 [details]
Patch, v2
Comment 3 Rob Buis 2022-02-23 04:14:27 PST
Comment on attachment 452893 [details]
Patch, v2

View in context: https://bugs.webkit.org/attachment.cgi?id=452893&action=review

> Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:3
> + * Copyright (C) 2009 Google, Inc.

Does this need Igalia 2022 copyright?

> Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:30
> +class RenderSVGTransformableContainer final : public RenderSVGContainer {

Is this really final? If so why do we have a method that is override and not final?

> Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:46
> +    FloatPoint extraContainerTranslation() const;

extra sounds not so nice, additional?

> Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:50
> +    void updateFromStyle() final;

Probably better to separate the methods and member vars below.
Comment 4 Radar WebKit Bug Importer 2022-03-01 01:03:36 PST
<rdar://problem/89607181>
Comment 5 Nikolas Zimmermann 2022-03-04 04:26:20 PST
(In reply to Rob Buis from comment #3)
> Comment on attachment 452893 [details]
> Patch, v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=452893&action=review
> 
> > Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:3
> > + * Copyright (C) 2009 Google, Inc.
> 
> Does this need Igalia 2022 copyright?
Fixed, also 2020 and 2021.

> 
> > Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:30
> > +class RenderSVGTransformableContainer final : public RenderSVGContainer {
> 
> Is this really final? If so why do we have a method that is override and not
> final?
Good question in general. Do we have rules/preferences for that?
Marking a class as final means that you cannot inherit from it anymore, a stricter constraint than imposing final/override qualifiers per function. Does that mean we automatically should rewrite s/override/final/ on all methods, because the class is marked as final? Here it makes sense, just wondering about the general preferences.

Thanks.

> 
> > Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:46
> > +    FloatPoint extraContainerTranslation() const;
> 
> extra sounds not so nice, additional?
Fixed.

> 
> > Source/WebCore/rendering/svg/RenderSVGTransformableContainer.h:50
> > +    void updateFromStyle() final;
> 
> Probably better to separate the methods and member vars below.
Fixed.
Comment 6 Nikolas Zimmermann 2022-03-04 04:26:46 PST
Created attachment 453833 [details]
Patch, v3
Comment 7 Nikolas Zimmermann 2022-03-07 03:16:49 PST
Committed r290880 (248111@trunk): <https://commits.webkit.org/248111@trunk>