Bug 237024

Summary:

[LBSE] Begin layer-aware RenderSVGTransformableContainer implementation

Product:

WebKit

Reporter:

Nikolas Zimmermann <zimmermann>

Component:

SVG

Assignee:

Nikolas Zimmermann <zimmermann>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, rbuis, sabouhallawa, schenney, sergio, webkit-bug-importer, zimmermann

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

237023

Bug Blocks:

90738

Attachments:

Description	Flags
Patch	ews-feeder: commit-queue-
Patch, v2	none
Patch, v3	rbuis: review+, ews-feeder: commit-queue-

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>