Closed Bug 1090934 Opened 5 years ago Closed 5 years ago

Calculate the bounds of SVG line elements using Math

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: jwatt, Assigned: longsonr)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 6 obsolete files)

No description provided.
Blocks: 936338
Are you working on this Jonathan?
No, feel free to take it.
Attached patch line.txt (obsolete) — Splinter Review
Assignee: jwatt → longsonr
Attachment #8527352 - Flags: review?(jwatt)
Attachment #8527352 - Attachment is obsolete: true
Attachment #8527352 - Flags: review?(jwatt)
Attached patch the easy cases (obsolete) — Splinter Review
Attachment #8532818 - Flags: review?(jwatt)
Attachment #8532818 - Attachment is obsolete: true
Attachment #8532818 - Flags: review?(jwatt)
Attachment #8532822 - Flags: review?(jwatt)
Attached patch fixes (obsolete) — Splinter Review
Attachment #8532822 - Attachment is obsolete: true
Attachment #8532822 - Flags: review?(jwatt)
Attachment #8532897 - Flags: review?(jwatt)
Attached patch mozilla style fixes (obsolete) — Splinter Review
Attachment #8532897 - Attachment is obsolete: true
Attachment #8532897 - Flags: review?(jwatt)
Attachment #8532899 - Flags: review?(jwatt)
Attached patch also cope with linecap="square" (obsolete) — Splinter Review
Attachment #8532899 - Attachment is obsolete: true
Attachment #8532899 - Flags: review?(jwatt)
Attachment #8532928 - Flags: review?(jwatt)
Attached patch math.txtSplinter Review
Attachment #8532928 - Attachment is obsolete: true
Attachment #8532928 - Flags: review?(jwatt)
Attachment #8532957 - Flags: review?(jwatt)
Comment on attachment 8532957 [details] [diff] [review]
math.txt

Review of attachment 8532957 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/svg/SVGLineElement.cpp
@@ +144,5 @@
> +  if (!hasStroke) {
> +    *aBounds = Rect(aTransform * Point(x1, y1), Size());
> +    aBounds->ExpandToEnclose(aTransform * Point(x2, y2));
> +    return true;
> +  }

Seems like this is valid whether or not !aTransform.IsRectilinear() so this could happen at the top, and just be if |aStrokeWidth <= 0|, avoiding the need for |hasStroke|.

@@ +146,5 @@
> +    aBounds->ExpandToEnclose(aTransform * Point(x2, y2));
> +    return true;
> +  }
> +
> +  if (aCapStyle == CapStyle::ROUND) {

You could then move the !aTransform.IsRectilinear() check in here.

@@ +166,5 @@
> +      Float ratio = aStrokeWidth / 2.f / length;
> +      xDelta = ratio * (y2 - y1);
> +      yDelta = ratio * (x2 - x1);
> +    }
> +  } else {

MOZ_ASSERT(aCapStyle == CapStyle::SQUARE) here.
Attachment #8532957 - Flags: review?(jwatt) → review+
https://hg.mozilla.org/mozilla-central/rev/94c12d733e3b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.