Closed Bug 1117869 Opened 5 years ago Closed 5 years ago

Add move constructors and assignment operators for nsRegion

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file)

This should make things more cheap.
Attachment #8544085 - Flags: review?(botond)
Assignee: nobody → jmuizelaar
Comment on attachment 8544085 [details] [diff] [review]
Add move constructors and assignment operators for nsRegion

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

r+ with the calls to pixman_region32_init fixed.

::: gfx/src/nsRegion.h
@@ +61,4 @@
>                                                                            aRect.width,
>                                                                            aRect.height); }
>    nsRegion (const nsRegion& aRegion) { pixman_region32_init(&mImpl); pixman_region32_copy(&mImpl,aRegion.Impl()); }
> +  nsRegion (nsRegion&& aRegion) { mImpl = aRegion->mImpl; pixman_region32_init(&aRegion); }

Presumably you meant to call 'pixman_region32_init(&aRegion->mImpl)' here.

@@ +61,5 @@
>                                                                            aRect.width,
>                                                                            aRect.height); }
>    nsRegion (const nsRegion& aRegion) { pixman_region32_init(&mImpl); pixman_region32_copy(&mImpl,aRegion.Impl()); }
> +  nsRegion (nsRegion&& aRegion) { mImpl = aRegion->mImpl; pixman_region32_init(&aRegion); }
> +  nsRegion& operator = (nsRegion&& aRegion) { pixman_region_fini(&mImpl); mImpl = aRegion->mImpl; pixman_region32_init(&aRegion); }

And here.
Attachment #8544085 - Flags: review?(botond) → review+
Jeff, do you have measurements that prompted this change? I'd be interested to see them. Thanks.
Flags: needinfo?(jmuizelaar)
https://hg.mozilla.org/mozilla-central/rev/92382656739f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
(In reply to Nicholas Nethercote [:njn] from comment #2)
> Jeff, do you have measurements that prompted this change? I'd be interested
> to see them. Thanks.

Nope. It's just low hanging fruit that I knew we had around. I don't expect it to have a very large change, but every bit helps.
Flags: needinfo?(jmuizelaar)
You need to log in before you can comment on or make changes to this bug.