1117869 - Add move constructors and assignment operators for nsRegion

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Add move constructors and assignment operators for nsRegion

This should make things more cheap.

Comment 1

[Botond Ballo [:botond]]

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.

Comment 2

[Nicholas Nethercote [inactive]]

Jeff, do you have measurements that prompted this change? I'd be interested to see them. Thanks.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/92382656739f

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(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.