Closed
Bug 693319
Opened 13 years ago
Closed 12 years ago
Factor out common code from RasterImage checking whether flags match
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: khuey, Assigned: khuey)
References
Details
Attachments
(1 file)
|
(deleted),
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
RasterImage has some code that checks whether or not the flags it's handed match the flags it was decoded with and if not, whether another decode can be done. This code exists in triplicate and should be factored out.
|
Assignee | |
Comment 1•13 years ago
|
||
|
||
Comment 2•13 years ago
|
||
Comment on attachment 566011 [details] [diff] [review]
Patch
>+ bool TryToChangeFlags(PRUint32 aNewFlags);
Looks great except for the name. In particular, it doesn't specify the type of flags (decode flags), and we might not even be changing them at all.
ApplyDecodeFlags, maybe? Use your judgement here.
r=bholley with that
Attachment #566011 -
Flags: review?(bobbyholley+bmo) → review+
|
|
Assignee | |
Comment 3•13 years ago
|
||
Comment on attachment 566011 [details] [diff] [review]
Patch
Review of attachment 566011 [details] [diff] [review]:
-----------------------------------------------------------------
::: modules/libpr0n/src/RasterImage.cpp
@@ +854,5 @@
>
> +bool
> +RasterImage::TryToChangeFlags(PRUint32 aNewFlags)
> +{
> + if (mFrameDecodeFlags == aNewFlags & DECODE_FLAGS_MASK)
That wants to be if (mFrameDecodeFlags == (aNewFlags & DECODE_FLAGS_MASK)), of course ..
/me whistles
|
|
Assignee | |
Comment 4•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in
before you can comment on or make changes to this bug.
Description
•