code review 13407045: go.image/bmp: add Encode

bmp/writer.go

 // Copyright 2013 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package bmp
 
 import (
         "encoding/binary"
         "image"
         "io"
@@ skipping 10 matching lines @@
         colorPlane      uint16
         bpp             uint16
         compression     uint32
         imageSize       uint32
         xPixelsPerMeter uint32
         yPixelsPerMeter uint32
         colorUse        uint32
         colorImportant  uint32
 }
 
-func encodePaletted(w io.Writer, m image.Image, pix []uint8, stride, step int) error {
-        if stride < step {
-                padding := make([]byte, step-stride)
-                for y := m.Bounds().Dy() - 1; y >= 0; y-- {
-                        min := y*stride + 0
-                        max := y*stride + stride

[nigeltao, 2013/09/09 07:18:26]

This isn't right. Again, this can panic if width < stride.

[chai2010, 2013/09/09 07:42:41]

Done.

-                        if _, err := w.Write(pix[min:max]); err != nil {
-                                return err
-                        }
+func encodePaletted(w io.Writer, pix []uint8, dx, dy, stride, step int) error {
+        var padding []byte
+        if dx < step {
+                padding = make([]byte, step-dx)
+        }
+        for y := dy - 1; y >= 0; y-- {
+                min := y*stride + 0
+                max := y*stride + dx
+                if _, err := w.Write(pix[min:max]); err != nil {
+                        return err
+                }
+                if padding != nil {
                         if _, err := w.Write(padding); err != nil {
-                                return err
-                        }
-                }
-        } else {
-                for y := m.Bounds().Dy() - 1; y >= 0; y-- {
-                        min := y*stride + 0
-                        max := y*stride + step

[nigeltao, 2013/09/09 07:18:26]

This will panic if width < step && step <= stride.

[chai2010, 2013/09/09 07:42:41]

Done.

-                        if _, err := w.Write(pix[min:max]); err != nil {
                                 return err
                         }
                 }
         }
         return nil
 }
 
-func encodeRGBA(w io.Writer, m image.Image, pix []uint8, stride, step int) error {
+func encodeRGBA(w io.Writer, pix []uint8, dx, dy, stride, step int) error {
         buf := make([]byte, step)
-        for y := m.Bounds().Dy() - 1; y >= 0; y-- {
+        for y := dy - 1; y >= 0; y-- {
                 min := y*stride + 0
-                max := y*stride + m.Bounds().Dx()*4
+                max := y*stride + dx*4
                 off := 0
                 for i := min; i < max; i += 4 {
                         buf[off+2] = pix[i+0]
                         buf[off+1] = pix[i+1]
                         buf[off+0] = pix[i+2]
                         off += 3
                 }
                 if _, err := w.Write(buf); err != nil {
                         return err
                 }
         }
         return nil
 }
 
 func encode(w io.Writer, m image.Image, step int) error {
+        b := m.Bounds()
         buf := make([]byte, step)
-        for y := m.Bounds().Dy() - 1; y >= 0; y-- {
+        for y := b.Max.Y - 1; y >= b.Min.Y; y-- {
                 off := 0
-                for x := 0; x < m.Bounds().Dx(); x++ {
+                for x := b.Min.X; x < b.Max.X; x++ {
                         r, g, b, _ := m.At(x, y).RGBA()
                         buf[off+2] = byte(r >> 8)
                         buf[off+1] = byte(g >> 8)
                         buf[off+0] = byte(b >> 8)
                         off += 3
                 }
                 if _, err := w.Write(buf); err != nil {
                         return err
                 }
         }
         return nil
 }
 
 // Encode writes the image m to w in BMP format.
 func Encode(w io.Writer, m image.Image) error {
+        d := m.Bounds().Size()
         h := &header{
                 sigBM:         [2]byte{'B', 'M'},
                 fileSize:      14 + 40,
                 pixOffset:     14 + 40,
                 dibHeaderSize: 40,
-                width:         uint32(m.Bounds().Dx()),
-                height:        uint32(m.Bounds().Dy()),
+                width:         uint32(d.X),
+                height:        uint32(d.Y),
                 colorPlane:    1,
         }
 
         var step int
         var palette []byte
         switch m := m.(type) {
         case *image.Gray:
-                step = (m.Bounds().Dx() + 3) &^ 3
+                step = (d.X + 3) &^ 3
                 palette = make([]byte, 1024)
                 for i := 0; i < 256; i++ {
                         palette[i*4+0] = uint8(i)
                         palette[i*4+1] = uint8(i)
                         palette[i*4+2] = uint8(i)
                         palette[i*4+3] = 0xFF
                 }
-                h.imageSize = uint32(m.Bounds().Dy() * step)
+                h.imageSize = uint32(d.Y * step)
                 h.fileSize += uint32(len(palette)) + h.imageSize
                 h.pixOffset += uint32(len(palette))
                 h.bpp = 8
 
         case *image.Paletted:
-                step = (m.Bounds().Dx() + 3) &^ 3
+                step = (d.X + 3) &^ 3
                 palette = make([]byte, 1024)
                 for i := 0; i < len(m.Palette) && i < 256; i++ {
                         r, g, b, _ := m.Palette[i].RGBA()
                         palette[i*4+0] = uint8(b >> 8)
                         palette[i*4+1] = uint8(g >> 8)
                         palette[i*4+2] = uint8(r >> 8)
                         palette[i*4+3] = 0xFF
                 }
-                h.imageSize = uint32(m.Bounds().Dy() * step)
+                h.imageSize = uint32(d.Y * step)
                 h.fileSize += uint32(len(palette)) + h.imageSize
                 h.pixOffset += uint32(len(palette))
                 h.bpp = 8
         default:
-                step = (3*m.Bounds().Dx() + 3) &^ 3
-                h.imageSize = uint32(m.Bounds().Dy() * step)
+                step = (3*d.X + 3) &^ 3
+                h.imageSize = uint32(d.Y * step)
                 h.fileSize += h.imageSize
                 h.bpp = 24
         }
 
         if err := binary.Write(w, binary.LittleEndian, h); err != nil {
                 return err
         }
-        if err := binary.Write(w, binary.LittleEndian, palette); err != nil {

[nigeltao, 2013/09/09 07:18:26]

Guard this by
if palette != nil

[chai2010, 2013/09/09 07:42:41]

Done.

-                return err
+        if palette != nil {
+                if err := binary.Write(w, binary.LittleEndian, palette); err != nil {
+                        return err
+                }
         }
 
         switch m := m.(type) {
         case *image.Gray:
-                return encodePaletted(w, m, m.Pix, m.Stride, step)
+                return encodePaletted(w, m.Pix, d.X, d.Y, m.Stride, step)
         case *image.Paletted:
-                return encodePaletted(w, m, m.Pix, m.Stride, step)
+                return encodePaletted(w, m.Pix, d.X, d.Y, m.Stride, step)
         case *image.RGBA:
-                return encodeRGBA(w, m, m.Pix, m.Stride, step)
+                return encodeRGBA(w, m.Pix, d.X, d.Y, m.Stride, step)
         }
         return encode(w, m, step)
 }