Closed
Bug 988818
Opened 11 years ago
Closed 11 years ago
Stop creating a Thebes (gfxASurface) backed gfxContext in nsSVGImageFrame
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: jwatt, Assigned: jwatt)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
(deleted),
patch
|
longsonr
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
longsonr
:
review+
|
Details | Diff | Splinter Review |
We should stop creating a Thebes (gfxASurface) backed gfxContext in nsSVGImageFrame.
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #8397779 -
Flags: review?(longsonr)
Comment 2•11 years ago
|
||
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...
Updated•11 years ago
|
Attachment #8397779 -
Flags: review?(longsonr) → review-
Assignee | ||
Comment 3•11 years ago
|
||
(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).
Assignee | ||
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8399169 -
Flags: review?(longsonr)
Updated•11 years ago
|
Attachment #8399169 -
Flags: review?(longsonr) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•