Closed
Bug 370942
Opened 18 years ago
Closed 17 years ago
Remove non-Cairo from jpeg decoder and optimize loop
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha5
People
(Reporter: alfredkayser, Assigned: alfredkayser)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
* Remove the non-Cairo code
* Loop optimized from 20 CPU instructions to 13.
* Only query img interface when really needed
* Use GFX_PACKED_PIXEL for pixel packing
Flags: blocking1.9?
Attachment #255716 -
Flags: review?(pavlov)
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Assignee | ||
Comment 1•18 years ago
|
||
Simple loop optimization and use GFX_PACKED_PIXEL macro
Attachment #255716 -
Attachment is obsolete: true
Attachment #256695 -
Flags: review?(tor)
Attachment #255716 -
Flags: review?(pavlov)
Comment on attachment 256695 [details] [diff] [review]
V2: updated to current trunk
>- for (PRUint32 i=0; i < mInfo.output_width; ++i) {
>- PRUint8 r = *j1++;
>- PRUint8 g = *j1++;
>- PRUint8 b = *j1++;
>- *ptrOutputBuf++ = (0xFF << 24) | (r << 16) | (g << 8) | b;
>+ for (PRUint32 i=mInfo.output_width; i>0; --i) {
>+ *ptrOutputBuf++ = GFX_PACKED_PIXEL(0xFF, j1[0], j1[1], j1[2]);
>+ j1+=3;
> }
Why did you flip the loop control direction?
Assignee | ||
Comment 3•18 years ago
|
||
This is the same also done for the PNG decoder:
http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/decoders/png/nsPNGDecoder.cpp414 414 for (PRUint32 x=iwidth; x>0; --x) {
415 *cptr32++ = GFX_PACKED_PIXEL(0xFF, line[0], line[1], line[2]);
416 line += 3;
417 }
Because the walker value (x) is not needed in the loop itself, using a flipped loop allows for better (smaller and faster) looping code.
This reduces the looping code from 20 instructions to 13, and removes the need to check 'mInfo.output_width' in the loop.
Comment on attachment 256695 [details] [diff] [review]
V2: updated to current trunk
One would hope that the compiler would already do that sort of optimization, but probably can't hurt.
Attachment #256695 -
Flags: review?(tor) → review+
Assignee | ||
Comment 5•18 years ago
|
||
Attachment #256695 -
Attachment is obsolete: true
Attachment #256838 -
Flags: superreview?(pavlov)
Comment 6•18 years ago
|
||
Comment on attachment 256838 [details] [diff] [review]
V3: Also update the Makefile.in to include thebes
> REQUIRES = xpcom \
> string \
> gfx \
>+ thebes \
please use tabs in makefiles.
sr= with that change
Attachment #256838 -
Flags: superreview?(pavlov) → superreview+
Assignee | ||
Comment 7•18 years ago
|
||
Who can do the checkin of this?
Attachment #256838 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Whiteboard: [wanted-1.9] → [wanted-1.9][checkin needed]
Updated•17 years ago
|
Assignee: nobody → alfredkayser
Comment 8•17 years ago
|
||
Attachment #260564 -
Attachment is obsolete: true
Comment 9•17 years ago
|
||
Checking in nsJPEGDecoder.cpp;
/cvsroot/mozilla/modules/libpr0n/decoders/jpeg/nsJPEGDecoder.cpp,v <-- nsJPEGDecoder.cpp
new revision: 1.72; previous revision: 1.71
done
The makefile.in change was already in.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Whiteboard: [wanted-1.9][checkin needed] → [wanted-1.9]
Target Milestone: --- → mozilla1.9alpha5
Comment 11•17 years ago
|
||
This may have caused orange on qm-xserve01 dep unit test, I'm waiting for it to cycle one more time to see if the box cures itself.
Comment 12•17 years ago
|
||
Nope, looks like it was just a glitch with the box.
Updated•17 years ago
|
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
You need to log in
before you can comment on or make changes to this bug.
Description
•