Fix non-unified-build errors for dom/canvas

RESOLVED FIXED in Firefox 50

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

50 Branch
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Assignee)

Description

2 years ago
It's that time again!
(Assignee)

Comment 1

2 years ago
Created attachment 8773647 [details]
Bug 1288649 - Fix non-unified-build errors. -

Review commit: https://reviewboard.mozilla.org/r/66408/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66408/
Attachment #8773647 - Flags: review?(ethlin)
Attachment #8773648 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

2 years ago
Created attachment 8773648 [details]
Bug 1288649 - Use mfbt IsPowerOfTwo funcs in gfx/. -

While fixing non-unified-build errors in dom/canvas, I started hitting a
static_assert that we were calling IsPowerOfTwo with a signed type. It
turns out we have at least three copies of IsPowerOfTwo() in the tree.
Let's drop the non-mfbt ones.

Review commit: https://reviewboard.mozilla.org/r/66410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66410/
(Assignee)

Comment 3

2 years ago
Created attachment 8773649 [details]
Bug 1288649 - 2-arg ctors shouldn't be explicit.

Review commit: https://reviewboard.mozilla.org/r/66412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66412/
Comment on attachment 8773648 [details]
Bug 1288649 - Use mfbt IsPowerOfTwo funcs in gfx/. -

https://reviewboard.mozilla.org/r/66410/#review63180

::: gfx/thebes/gfxUtils.h:299
(Diff revision 1)
>  /**
> - * Returns the first integer greater than or equal to |aNumber| which is a
> + * Returns the first integer greater than |aNumber| which is a power of two.
> - * power of two. Undefined for |aNumber| < 0.
>   */
> -static inline int
> +static int
>  NextPowerOfTwo(int aNumber)

Although the name for this function is not great. The semantics are the ones that we want. i.e. NextPowerOfTwo(4) == 4.

It would be better to change the existing callers to use RoundUpPow2
Attachment #8773648 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 5

2 years ago
Created attachment 8773972 [details]
Bug 1288649 - Remove NextPowerOfTwo. -

Review commit: https://reviewboard.mozilla.org/r/66602/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66602/
Attachment #8773972 - Flags: review?(jmuizelaar)

Comment 8

2 years ago
Comment on attachment 8773647 [details]
Bug 1288649 - Fix non-unified-build errors. -

https://reviewboard.mozilla.org/r/66408/#review63462
Attachment #8773647 - Flags: review?(ethlin) → review+
Comment on attachment 8773972 [details]
Bug 1288649 - Remove NextPowerOfTwo. -

https://reviewboard.mozilla.org/r/66602/#review63634
Attachment #8773972 - Flags: review?(jmuizelaar) → review+

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/91f0e0f0e398
https://hg.mozilla.org/mozilla-central/rev/cb620c9d9139
https://hg.mozilla.org/mozilla-central/rev/eb943039fc6d
https://hg.mozilla.org/mozilla-central/rev/19686b2399b5
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.