Closed Bug 988818 Opened 11 years ago Closed 11 years ago

Stop creating a Thebes (gfxASurface) backed gfxContext in nsSVGImageFrame

Categories

(Core :: SVG, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: jwatt, Assigned: jwatt)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

We should stop creating a Thebes (gfxASurface) backed gfxContext in nsSVGImageFrame.
Attached patch patch (deleted) — Splinter Review
Attachment #8397779 - Flags: review?(longsonr)
Comment on attachment 8397779 [details] [diff] [review] patch >+ * Given a float, constrains its value to be between nscoord_MIN and nscoord_MAX. >+ * >+ * @param aVal The value to constrain (in/out) >+ */ >+static void ConstrainToCoordValues(float& aVal) >+{ >+ if (aVal <= nscoord_MIN) >+ aVal = nscoord_MIN; >+ else if (aVal >= nscoord_MAX) >+ aVal = nscoord_MAX; use mozilla::clamped to implement? Maybe this shouldn't be static given that nsSVGUtils::TransformOuterSVGPointToChildFrame could use it. >+} >+ >+static void ConstrainToCoordValues(float& aStart, float& aSize) >+{ >+ float max = aStart + aSize; >+ >+ // Clamp the end points to within nscoord range >+ ConstrainToCoordValues(aStart); >+ ConstrainToCoordValues(max); So we just truncate here but bring both the endpoints in together below. That seems strange. Why? >+ >+ aSize = max - aStart; >+ // If the width if still greater than the max nscoord, then bring both If the width **is** still...
Attachment #8397779 - Flags: review?(longsonr) → review-
(In reply to Robert Longson from comment #2) > So we just truncate here but bring both the endpoints in together below. > That seems strange. Why? I'll talk about the x-axis properties here, but obviously the same applies for the y-axis properties. There are three properties of the rect that we care about getting in range: x, xmost (aka the right edge) and width. Obviously the bounds of the rect have to be within {nscoord_MIN,nscoord_MAX}, so we clamp x and xmost to that range. That done, we still need width to be less than or equal to nscoord_MAX. Rather than simply clamp, we pull in the edges to keep the rect centered. From my perspective I'm doing that mostly because that's what we've historically done in these situations, but also because it seems the most sane thing to me (particularly for SVG where we're not flowing things and aligning them to an edge).
Maybe it helps to think of this in terms of an object of size {x,0,width,1}. (y and height being given simple values so we can simplify our discussion to just consider the x-axis.) There are several scenarios where we need to change the bounds of the object: 1) If the object is positioned fully outside {nscoord_MIN,nscoord_MAX} then presumably we can agree that it should probably become zero sized. 2) If the object is fully within {nscoord_MIN,nscoord_MAX} but its width is greater than nscoord_MAX then hopefully we can agree that reducing its width to nscoord_MAX while keeping it centered is likely to produce the least bad result most of the time? 3) If the object is overlapping x=nscoord_MIN or x=nscoord_MAX with a width of less than nscoord_MAX then what should we do? We could pull in the edges until width<nscoord_MAX but that would make an object that should have partially been in range zero sized if more than half of it was outside {nscoord_MIN,nscoord_MAX}. I think it's better to clamp its bounds to keep the nsRect covering as much of the area as the original gfxRect covered. That way there's a better chance of a visual indication of range loss. 4) If the object is overlapping x=nscoord_MIN or x=nscoord_MAX with a width greater than nscoord_MAX I think we should do the same as (3), _then_ pull in the edges. Again that keeps the resulting nsRect covering as much of the original gfxRect as possible, while still centering when the width is too big.
Attached patch patch (deleted) — Splinter Review
Attachment #8399169 - Flags: review?(longsonr)
Attachment #8399169 - Flags: review?(longsonr) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Depends on: 993443
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: