Closed Bug 463465 Opened 12 years ago Closed 11 years ago

Artifacts in APNG frames

Categories

(Core :: ImageLib, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: tracy, Assigned: glennrp+bmo)

References

(Blocks 1 open bug, )

Details

(Keywords: regression, Whiteboard: [FFT3.5])

Attachments

(2 files, 2 obsolete files)

The test case for fixed bug 420416 has regressed in nightly trunk builds.

the second image has a smear
Flags: in-litmus+
Duplicate of this bug: 469169
This is closely related to bug #433047 if not a dupe.
Assignee: nobody → glennrp
Status: NEW → ASSIGNED
also takes care of non-progressive reading
Attachment #377927 - Attachment is obsolete: true
Comment on attachment 377978 [details] [diff] [review]
Patch to clear the prev_row before each APNG frame (v1)

This does not need to wait for libpng-1.2.37, which does not address this problem.
Attachment #377978 - Flags: review?(joe)
Would someone with access to the tri-server please make a tri-server build with this patch?
Done - it's building now. I'll update when it's done.
https://build.mozilla.org/tryserver-builds/jdrew@mozilla.com-glennrp-apng-v1/ - linux is done, others will be showing up. I haven't looked at unittest results, though.
The test case for bug #420416 still has the artifacts with the triserver Windows build.  I haven't tried the other two builds.  The test case for bug #433047 also still displays artifacts.
The test case for this bug looks OK, however.
This (v1) patch will be included in the upcoming libpng-1.2.37 release that's due out June 4th.
Blocks: 495609
Depends on: 492200
Moved memset of prev_row into png_read_reinit()

@joe: please make another triserver build with this patch together with the patch from bug #397593
Attachment #377978 - Attachment is obsolete: true
Attachment #377978 - Flags: review?(joe)
I believe the purpose of the code that the patch removes was to save CPU cycles in blending opaque images.  Throwing a png_error() seems an excessive response; a png_warning() is more appropriate.  Also the test was incorrect because it did not account for the possibility of a tRNS chunk in truecolor or grayscale images.  Furthermore, the test rejects indexed-color PNGs but throws an error saying it is rejecting a truecolor PNG.  Instead, we will simply ignore PNG_BLEND_OP_OVER in opaque images and use the simpler PNG_BLEND_OP_SOURCE, which produces the same result for opaque images, instead.
Sorry, the previous comment belongs in bug #441971.
The test case mentioned in the description (solar flare image) looks fine with Joe's latest triserver build, which includes the v2 patch.  The simple TEST 1/TEST 2 image also looks fine.
Attachment #380718 - Flags: superreview?(tor)
Attachment #380718 - Flags: review?(joe)
They look fine on Windows XP.  I am not in position to try the other platforms.
The v2 patch is incorporated in the libpng-1.2.37 patch (bug #492200)
If this one gets checked in before libpng-1.2.37 then I'll revise
the libpng-1.2.37 patch to avoid merge conflict.  If libpng patch gets
checked in first then this one will become obsolete.
Attachment #380718 - Flags: superreview?(vladimir)
Attachment #380718 - Flags: superreview?(tor)
Attachment #380718 - Flags: review?(joe)
Attachment #380718 - Flags: review+
Attachment #380718 - Flags: superreview?(vladimir) → superreview+
Attached image Screenshot
Here is a screenshot of the issue.
Adding [FFT3.5] in whiteboard.  Came across this issue during RC1 testing
Whiteboard: [FFT3.5]
Can this one be approved for check-in?
Depends on: 504805
Fixed by landing of bug 504805.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Attachment #380718 - Flags: approval1.9.2?
Attachment #380718 - Flags: approval1.9.1.7?
Attachment #380718 - Flags: approval1.9.0.17?
Comment on attachment 380718 [details] [diff] [review]
Patch to clear the prev_row before each APNG frame (v2)

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.
Attachment #380718 - Flags: approval1.9.2?
Attachment #380718 - Flags: approval1.9.1.7?
Attachment #380718 - Flags: approval1.9.0.17?
Attachment #380718 - Flags: approval1.9.0.17-
You need to log in before you can comment on or make changes to this bug.