Closed
Bug 886548
Opened 11 years ago
Closed 11 years ago
Replace non-hit-testing-related SetupCairoStrokeHitGeometry calls with SetupCairoStrokeGeometry
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
INVALID
People
(Reporter: dholbert, Assigned: dholbert)
Details
We have several calls to nsSVGUtils::SetupCairoStrokeHitGeometry for functions that aren't hit-testing-specific. nsSVGUtils::SetupCairoStrokeHitGeometry is itself just a thin wrapper for nsSVGUtils::SetupCairoStrokeGeometry, with a "SetDash" call added (presumably so that hit-testing on the between-dash region works correctly). So: filing this bug on doing s/SetupCairoStrokeHitGeometry/SetupCairoStrokeGeometry/ where there no hit-testing going on.
Comment 1•11 years ago
|
||
nsSVGUtils::SetupCairoStrokeGeometry is only called in nsSVGPathGeometryFrame::GetBBoxContribution it skips the SetDash there as whether dash styles can't affect the bounding box, at least not with the approximation based code we currently have. We could just have the one call and always call SetDash though which would be simpler and only slightly less efficient. Alternatively s/SetupCairoStrokeGeometry/SetupCairoStrokeBBoxGeometry/ and s/SetupCairoStrokeHitGeometry/SetupCairoStrokeGeometry/ perhaps.
Assignee | ||
Comment 2•11 years ago
|
||
(I was particularly eying the calls to SetupCairoStrokeHitGeometry in nsSVGGlyphFrame.cpp and nsSVGTextFrame2.cpp, which don't appear to need to know about dash geometry -- at least, if I do s/SetupCairoStrokeHitGeometry/SetupCairoStrokeGeometry/ there, SVG reftests still pass, modulo some minor machine-specific fuzziness.)
Assignee | ||
Comment 3•11 years ago
|
||
Exploratory Try run: https://tbpl.mozilla.org/?tree=Try&rev=0dd6c25cb647
Assignee | ||
Comment 4•11 years ago
|
||
Try seemed to like this, but it does actually break a reftest -- though it breaks the reftest and the reference case in the same way, so it still passes. :-/ It breaks the rendering of: http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/stroke-dasharray-and-text-01.svg and http://mxr.mozilla.org/mozilla-central/source/layout/reftests/svg/stroke-dasharray-and-text-01-ref.svg to no longer have dashes. So it looks like we don't have any reftests or mochitests that test whether we successfully apply stroke-dasharray to text. We should add some. (Probably easiest as "!=" reftests, with dashed text != normal text.) And this bug here is INVALID, since we need SetupCairoStrokeHitGeometry's SetDash call in these places after all. I may spin off a different bug to do the renaming that Robert suggested in comment 1, to make this clearer.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 5•11 years ago
|
||
(I filed bug 886611 for the renaming.)
You need to log in
before you can comment on or make changes to this bug.
Description
•