Bug 224482 - Make SVGElement::getBoundingBox retrieve bbox from RenderObject
Summary: Make SVGElement::getBoundingBox retrieve bbox from RenderObject
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-13 05:36 PDT by Rob Buis
Modified: 2021-04-14 11:16 PDT (History)
15 users (show)

See Also:

Attachments
Patch (3.21 KB, patch)
2021-04-13 05:37 PDT, Rob Buis 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (3.76 KB, patch)
2021-04-13 06:07 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (6.02 KB, patch)
2021-04-13 08:23 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2021-04-13 05:36:29 PDT 
Make SVGElement::getBoundingBox retrieve bbox from RenderObject.
Comment 1 Rob Buis 2021-04-13 05:37:57 PDT 
Created attachment 425865 [details]
Patch
Comment 2 Rob Buis 2021-04-13 06:07:03 PDT 
Created attachment 425867 [details]
Patch
Comment 3 Rob Buis 2021-04-13 08:23:35 PDT 
Created attachment 425873 [details]
Patch
Comment 4 zalan 2021-04-13 13:25:25 PDT 
Wouldn't this return incorrect rect for SVGPathElement?
Comment 5 Rob Buis 2021-04-13 13:39:09 PDT 
(In reply to zalan from comment #4)
> Wouldn't this return incorrect rect for SVGPathElement?

I do not think so, an SVGPathElement is a SVGGraphicsElement too. There should be no change in behaviour.
Comment 6 zalan 2021-04-13 15:22:54 PDT 
The difference is 
1. path().boundingRect() (SVGPathElement::getBBox -> RenderSVGPath::path().boundingRect()) vs. 
2. some cached path().boundingRect() value (through RenderSVGPath::objectBoundingBox() which returns m_fillBoundingBox which is populated at updateShapeFromElement by calling calculateObjectBoundingBox (which is indeed path().boundingRect()),
but I think it should be fine.
Comment 7 zalan 2021-04-13 15:40:27 PDT 
ResizeObservation::computeObservedSize (and some of the other call sites) are also "change in functionality" but let's consider them progressions.
Comment 8 Rob Buis 2021-04-13 23:52:14 PDT 
(In reply to zalan from comment #7)
> ResizeObservation::computeObservedSize (and some of the other call sites)
> are also "change in functionality" but let's consider them progressions.

Ah sorry, I missed this and the Path specific code, I thought there was only SVGLocatable::getBBox. Thanks for analysing and given it turned out green, the change should be safe.
Comment 9 EWS 2021-04-14 00:00:05 PDT 
Committed r275935 (236497@main): <https://commits.webkit.org/236497@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425873 [details].
Comment 10 Radar WebKit Bug Importer 2021-04-14 00:01:17 PDT 
<rdar://problem/76631485>
Comment 11 zalan 2021-04-14 11:16:48 PDT 
(In reply to Rob Buis from comment #8)
> (In reply to zalan from comment #7)
> > ResizeObservation::computeObservedSize (and some of the other call sites)
> > are also "change in functionality" but let's consider them progressions.
> 
> Ah sorry, I missed this and the Path specific code, I thought there was only
> SVGLocatable::getBBox. Thanks for analysing and given it turned out green,
> the change should be safe.
Thanks for fixing it.