Closed Bug 886548 Opened 11 years ago Closed 11 years ago

Replace non-hit-testing-related SetupCairoStrokeHitGeometry calls with SetupCairoStrokeGeometry

Categories

(Core :: SVG, defect)

defect
Not set
normal

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.
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.
(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.)
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
(I filed bug 886611 for the renaming.)
You need to log in before you can comment on or make changes to this bug.