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: encoder does not honor image bounds. #6635

Open
gopherbot opened this issue Oct 21, 2013 · 8 comments

Comments

@gopherbot
Copy link

commented Oct 21, 2013

by dan.pupius:

1. Open a image using gif.DecodeAll
2. Replace frame(s) with a `SubImage`, e.g. to make a square crop
3. Save image using gif.EncodeAll

What is the expected output?
Saved image should correctly show cropped region

What do you see instead?
Resultant image is incorrectly offset, see lucha.out.gif vs lucha.out.jpg

Which operating system are you using?
OSX

Which version are you using?  (run 'go version')
$ go version
go version devel +560ca6cc94b5 Sun Oct 20 18:29:15 2013 -0700 darwin/amd64

Please provide any additional information below.
I think the sImageDescriptor block in gif/writer.go needs to have the X/Y values
adjusted by the Min bounds of the first frame.

Attachments:

  1. gifs.go (966 bytes)
  2. lucha.gif (7598 bytes)
  3. lucha.out.jpg (17750 bytes)
  4. lucha.out.gif (6843 bytes)
@robpike

This comment has been minimized.

Copy link
Contributor

commented Oct 21, 2013

Comment 1:

Labels changed: added priority-later, packagebug, removed priority-triage.

Owner changed to @nigeltao.

Status changed to Accepted.

@gopherbot

This comment has been minimized.

Copy link
Author

commented Oct 21, 2013

Comment 2 by dan.pupius:

Here's a workaround, should anyone else hit this issue.  You have to create a new zero
based image for each frame and instead of using SubImage copy the crop region
35c36,41
<        im.Image[index] = frame.SubImage(dstBounds).(*image.Paletted)
---
>        // Creates a new image bounded at (0,0) and instead of creating a SubImage,
>        // copy the cropped region into a new image.
>        b := image.Rect(0, 0, dstBounds.Dx(), dstBounds.Dy())
>        pm := image.NewPaletted(b, frame.Palette)
>        draw.Draw(pm, b, frame, dstBounds.Min, draw.Src)
>        im.Image[index] = pm
@nigeltao

This comment has been minimized.

Copy link
Contributor

commented Oct 22, 2013

Comment 3:

I'll look at this after the 1.2 release. Feel free to ping this bug (after then) if I've
forgotten.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2013

Comment 4:

Labels changed: added go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 5:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Dec 4, 2013

Comment 6:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015

@urld

This comment has been minimized.

Copy link
Contributor

commented Mar 8, 2018

In go version go1.10 linux/amd64 the output looks like this: lucha.out.1.10.gif

This seems to be a legitimate output, since each Image Descriptor may have different dimensions and offsets from the Screen Descriptor (as long they stay within the bounds of Screen Descriptor).

@iWdGo

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2018

While investigating any link between GIF issues, the documentation does not specify that the original size of the GIF (image config) is kept after crop. You cannot update the config of the image (logical screen descriptor) after cropping and it is not set to the minimal size. On 1.11 beta 2, the GIF is valid with appropriate offset but the Logical Descriptor has the original size which is incorrect.

0: lucha.gif (original file before editing)
Logical screen descriptor : [300x300] and fields == 1000100
Image descriptor: offset [0x0] for image size == [300x300] and image fields == 0
1: luchafromissue.gif (as issue was filed)
Logical screen descriptor : [300x150] and fields == 0
Image descriptor: offset [0x75] for image size == [300x150] and image fields == 10000101
 decoding fails with  gif: frame bounds larger than image bounds

The original issue with adapted logical descriptor (image config) while keeping the offset.

2: lucha111beta2.gif (using latest beta release)
Logical screen descriptor : [300x300] and fields == 1000100
Image descriptor: offset [0x75] for image size == [300x150] and image fields == 0

Latest behaviour produces a correct GIF by retaining the original Logical Descriptor.

3: luchacroppedbyPaint3D.gif (edited using Paint3D on Windows 10)
Logical screen descriptor : [300x150] and fields == 1110000
Image descriptor: offset [0x0] for image size == [300x150] and image fields == 10000101

A simplified GIF structure without offset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.