Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image/gif: GIF encoder corrupts subimages #7792

Closed
danbathgate opened this issue Apr 15, 2014 · 5 comments
Closed

image/gif: GIF encoder corrupts subimages #7792

danbathgate opened this issue Apr 15, 2014 · 5 comments
Milestone

Comments

@danbathgate
Copy link

@danbathgate danbathgate commented Apr 15, 2014

What does 'go version' print?
go version devel +8e5787506b59 Tue Apr 15 15:52:23 2014 -0400 + linux/amd64

What steps reproduce the problem?
If possible, include a link to a program on play.golang.org.

1. Create an image in a GIF where the bounds of the Paletted image is different from the
backing pixel slice. For example, create an image using Paletted.SubImage() that is
narrower than the original image.
2. Attempt to encode this GIF.

Example (needs file access): http://play.golang.org/p/Ck6A7ih6Hv

What happened?
The frame created from the subimage looks corrupted (example in buggy.gif)

What should have happened instead?
The frame should look correct (example in fixed.gif)

Please provide any additional information below.
Looks like this is due to the "common mistake" described at
http://blog.golang.org/go-image-package#TOC_4. where it is assumed that the backing data
matches the dimensions of the image.

The encoder attempts to write the entire Px slice instead of respecting the image
bounds: http://golang.org/src/pkg/image/gif/writer.go?s=5197:5226#L236

The attached patch appears to fix it. I'd be happy to make a CL once I figure out the
review tools.

Attachments:

  1. buggy.gif (26883 bytes)
  2. fixed.gif (17539 bytes)
  3. gif_writer_subimage.patch (508 bytes)
@danbathgate
Copy link
Author

@danbathgate danbathgate commented Apr 16, 2014

Comment 1:

Note that the examples are animated GIFs (the issue tracker only shows the first frame
unless you download them). This can affect single frame GIFs as well, but I wanted to
make the examples clearly distinct from issue #6635 (image/gif: encoder does not honor
image bounds).
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 9, 2014

Comment 2:

Labels changed: added repo-main, release-go1.4.

@rsc
Copy link
Contributor

@rsc rsc commented Sep 15, 2014

Comment 3:

Nigel, will you get to this for 1.4? If not, postpone to 1.5 please.
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 17, 2014

Comment 4:

CL https://golang.org/cl/147730043 mentions this issue.
@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Sep 18, 2014

Comment 5:

This issue was closed by revision a291095.

Status changed to Fixed.

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015
@rsc rsc removed the release-go1.4 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
benchmark                    old ns/op     new ns/op     delta
BenchmarkEncode              8641055       8646829       +0.07%

Fixes golang#7792.

LGTM=r
R=r
CC=dbathgate, golang-codereviews
https://golang.org/cl/147730043
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
benchmark                    old ns/op     new ns/op     delta
BenchmarkEncode              8641055       8646829       +0.07%

Fixes golang#7792.

LGTM=r
R=r
CC=dbathgate, golang-codereviews
https://golang.org/cl/147730043
wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
benchmark                    old ns/op     new ns/op     delta
BenchmarkEncode              8641055       8646829       +0.07%

Fixes golang#7792.

LGTM=r
R=r
CC=dbathgate, golang-codereviews
https://golang.org/cl/147730043
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.