Closed Bug 1022059 Opened 8 years ago Closed 8 years ago

Rewrite MergeRects in a saner more like pixman style

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Details

Attachments

(1 file)

No description provided.
Attachment #8436210 - Flags: review?(matt.woodrow)
Comment on attachment 8436210 [details] [diff] [review]
Rewrite MergeRects in a saner more like pixman style

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

::: gfx/src/nsRegion.cpp
@@ +211,5 @@
> + * Yes, but not easily. We can even do it stablely
> + * but we don't need that property. */
> +#define MERGERECT(r) \
> +    do {                    \
> +      if (r->x1 <= x2) {    \

More comments! If 'r' overlaps the current rect, then expand the current rect to include it. Otherwise write the current rect out to tmpRect, and set r as the updated current rect.

@@ +242,5 @@
> +     * it to the the left most one */
> +    int x1;
> +    int x2;
> +    int y1 = r1->y1;
> +    int y2 = r2->y2;

Maybe make these const so it's clearer that they don't change

@@ +279,5 @@
> +    tmpRect->x1 = x1;
> +    tmpRect->x2 = x2;
> +    tmpRect->y1 = y1;
> +    tmpRect->y2 = y2;
> +    tmpRect++;

If you're going with macros, then you might as well avoid repeating this block of code using WRITE_RECT or something.
Attachment #8436210 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/94527ff59216
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.