@splace Thanks for the issue report.

When you say "images the same" under "what did you expect", can you clarify? There are 4 images, which ones did you expect to be the same?

the images are in the order the code produced them, they should all be the same since the image is not being modified at any point.

details....
the image (made from inline gif in base64) has transparency, when this is saved as a png or a gif the transparency is there in both files, but when the image is embedded in a new type, offsetImage, (still the same Image to the interface) then saving as png is fine, but saved as gif causes the transparency to be replaced by black. so it seems the gif encoder is in some way 'bypassing' the interface, being aware and varying what it does depending on the specific type.

The concrete type returned by image.Decode in your snippet is *image.Paletted.

I suspect this issue is happening because wrapping the *image.Paletted in a concrete type such as offsetImage makes the type assertion in gif.Encode no longer report positively:

/cc @robpike @nigeltao Do you think this is working as intended, or is this a bug in encoding/gif? Should it handle situations where the concrete type of the passed image.Image is not precisely *image.Paletted the same as if it were?

FYI this was encountered when i put together a simple image geometry transforming utility. (any supported format input, any output.) with the very simple technique of wrapping the Image and modifying the x and y of the method At(x,y) then calling the embedded Image. works as expected, except for this.

Yeah, it's a bug in encoding/gif, although not one that's trivial to fix. The

pm, ok := m.(*image.Paletted)
if !ok || len(pm.Palette) > opts.NumColors {
  etc
}

check should be something like

pm, _ := m.(*image.Paletted)
if pm == nil {
  // Either one or both of
  if pm2, ok := m.(image.PalettedImage) {
    // Create a new pm, based on pm2.
    // Note that image.PalettedImage is an interface; image.Paletted is a concrete type.
    // Yeah, the names are confusing. In hindsight, I'd do it differently.
  }
  // or
  if _, ok := m.ColorModel().(color.Palette); ok {
    // Create a new pm, based on m.
  }
}
if (pm == nil) || (len(pm.Palette) > opts.NumColors) {
  etc
}

But that's not great. The first approach might not be feasible if it requires your wrapper type to also or conditionally implement the image.PalettedImage interface.

The second approach would probably work. It won't be super-efficient to, for each pixel, call At and look up the color.Palette to turn it back into an index. But given the Go 1.x image interfaces that we have, it could be feasible.

@nigeltao, what do you suggest we do?

If somebody has the free time to fix the bug as I suggested (preferably the second approach), that's great.

I don't have the free time, though.

If nobody has the free time, then I think we just have to leave it as is for now. It's not a regression.

@nigeltao, I tried your second approach, and it works fine.

The first and second pictures are the results from @splace 's test.
The third pictures are the results from the test which contains offset.
Every gif picture has a transparent background.

My code is as follows,

  pm, _ := m.(*image.Paletted)
  if pm == nil {
    if cp, ok := m.ColorModel().(color.Palette); ok {
      pm = image.NewPaletted(b, cp)
      for i:=b.Min.X ; i < b.Max.X ; i++ {
        for j:=b.Min.Y ; j < b.Max.Y ; j++ {
          pm.Set(i,j, cp.Convert(m.At(i, j)))
          //pm.Set(i,j, m.At(i, j)) // also works 
        }
      }
    }
  }
  if (pm == nil) || (len(pm.Palette) > opts.NumColors) {
    // TODO: Pick a better sub-sample of the Plan 9 palette.
    pm = image.NewPaletted(b, palette.Plan9[:opts.NumColors])
    if opts.Quantizer != nil {
      pm.Palette = opts.Quantizer.Quantize(make(color.Palette, 0, opts.NumColors), m)
    }
    opts.Drawer.Draw(pm, b, m, image.ZP)
  }

Is this code what you expected?

Yeah, that looks OK, broadly speaking. I guess that it wasn't as complicated as I first feared. It's not super-efficient, but it will work, and we can optimize later.

There's some style nits (e.g. I'd call the variables x and y instead of i and j, and I'd also iterate y as the outer loop, not the inner one), but we can fix that in code review.

Change https://golang.org/cl/177377 mentions this issue: image/gif: transparency lost when encoding a wrapped *image.Paletted