Closed
Bug 714937
Opened 12 years ago
Closed 11 years ago
replace GFX_PACKED_PIXEL with an inline function
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: jrmuizel, Assigned: Luqman)
Details
(Whiteboard: [mentor=jrmuizel][lang=c++])
Attachments
(1 file, 5 obsolete files)
No description provided.
Reporter | ||
Comment 1•12 years ago
|
||
There's no need for GFX_PACKED_PIXEL to be a macro anymore and having it as an inline function will make things cleaner.
Whiteboard: [mentor=jrmuizel][lang=c++]
Assignee | ||
Comment 2•11 years ago
|
||
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 639149 [details] [diff] [review] Replace the GFX_PACKED_PIXEL macro with an inline function. Review of attachment 639149 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxColor.h @@ +107,5 @@ > +/** > + * Pack the 4 8-bit channels (A,R,G,B) > + * into a 32-bit packed premultiplied pixel. > + */ > +PRUint32 inline Use MOZ_ALWAYS_INLINE instead. @@ +118,5 @@ > + } else { > + return ((a) << 24) | > + (GFX_PREMULTIPLY(r,a) << 16) | > + (GFX_PREMULTIPLY(g,a) << 8) | > + (GFX_PREMULTIPLY(b,a)); GFX_PREMULTIPLY and GFX_PACKED_PIXEL_NO_PREMULTIPLY could also be changed to inline functions. But you can do that in other patches.
Attachment #639149 -
Flags: review-
Assignee | ||
Comment 4•11 years ago
|
||
Use MOZ_ALWAYS_INLINE.
Attachment #639149 -
Attachment is obsolete: true
Reporter | ||
Updated•11 years ago
|
Attachment #639152 -
Flags: review+
Assignee | ||
Comment 5•11 years ago
|
||
Replaced the other macros as well.
Attachment #639152 -
Attachment is obsolete: true
Attachment #639158 -
Flags: review?(jmuizelaar)
Reporter | ||
Updated•11 years ago
|
Attachment #639158 -
Flags: review?(jmuizelaar) → review+
![]() |
||
Updated•11 years ago
|
Comment 6•11 years ago
|
||
This patch is badly bitrotted (it doesn't apply at all anymore). Please rebase and attach a new patch. Also, please make sure that your patch follows the guidelines below so that all the necessary commit information is present within it. https://developer.mozilla.org/en-US/docs/Creating_a_patch_that_can_be_checked_in Also, bonus points if you post a link to a successful Try run with the patch! (If you don't, that's fine. I run patches myself before landing if I can't see that they already were)
Keywords: checkin-needed
Assignee | ||
Comment 7•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=055f068a1a03
Attachment #639158 -
Attachment is obsolete: true
Assignee | ||
Comment 8•11 years ago
|
||
Missed some OS X specific stuff so this should fix that. As for the android build failures, for some reason they don't seem to have MOZ_ALWAYS_INLINE defined.
Attachment #658748 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Builds on both android on os x now. https://tbpl.mozilla.org/?tree=Try&rev=cd2ac344c57d
Attachment #658767 -
Attachment is obsolete: true
Comment 10•11 years ago
|
||
Looks good on Try, thanks for sticking with it! I'll check this in today unless someone else beats me to it.
Keywords: checkin-needed
Comment 11•11 years ago
|
||
This might have been part of a shutdown timeout (see this log: <https://tbpl.mozilla.org/php/getParsedLog.php?id=15029553&tree=Mozilla-Inbound&full=1> for an example). I backed it out for now: https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e4864101e1
Keywords: checkin-needed
Comment 12•11 years ago
|
||
It wasn't. Re-landed. https://hg.mozilla.org/integration/mozilla-inbound/rev/a52b3e3632d5
Flags: in-testsuite-
Comment 13•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a52b3e3632d5
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Comment 14•11 years ago
|
||
Comment on attachment 658804 [details] [diff] [review] Replace GFX_PACKED_PIXEL/GFX_PACKED_PIXEL_NO_PREMULTIPLY/GFX_PREMULTIPLY macros Review of attachment 658804 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxColor.h @@ +97,5 @@ > * > * equivalent to (((c)*(a))/255) > */ > +PRUint8 MOZ_ALWAYS_INLINE gfxPreMultiply(PRUint8 c, PRUint8 a) { > + return GFX_DIVIDE_BY_255((c)*(a)); Would have been nice to get rid of the stray parens here, as in return GFX_DIVIDE_BY_255(c * a); , now this isn't a macro anymore.
You need to log in
before you can comment on or make changes to this bug.
Description
•