ev3dev/fb: add support for XRGB #78
Conversation
fb/xrgb.go
Outdated
| // NewXRGB returns a new XRGB image with the given bounds. | ||
| func NewXRGB(r image.Rectangle) *XRGB { | ||
| w, h := r.Dx(), r.Dy() | ||
| stride := 2 * w |
There was a problem hiding this comment.
Should this be 4 instead of 2?
fb/xrgb.go
Outdated
| return &XRGB{Pix: pix, Stride: stride, Rect: r}, nil | ||
| } | ||
|
|
||
| // XRGB is an in-memory image whose At method returns Pixel565 values. |
There was a problem hiding this comment.
It looks like the At method returns color.Color, not Pixel565
There was a problem hiding this comment.
This is fixed, but At has to return color.Color to satisfy the image.Image interface. The underlying concrete value is a PixelXRGB.
fb/xrgb.go
Outdated
| // Pix holds the image's pixels, as 32 bit XRGB | ||
| // values stored in little-endian order. | ||
| // The pixel at (x, y) is the four bytes at | ||
| // Pix[2*(x-Rect.Min.X) + (y-Rect.Min.Y)*Stride]. |
| } | ||
|
|
||
| // ColorModel returns the XRGB color model. | ||
| func (p *XRGB) ColorModel() color.Model { return RGB565Model } |
There was a problem hiding this comment.
Yes, The only difference between an XRGB and a color.RGBA is the absence of the A channel. The storage in the image's Pix field` is independent of the colour's representation here.
fb/xrgb.go
Outdated
| return PixelXRGB{R: p.Pix[i+2], G: p.Pix[i+1], B: p.Pix[i]} | ||
| } | ||
|
|
||
| // Set sets the color of the pixel565 at (x, y) to c. |
There was a problem hiding this comment.
Again, the mention of pixel565 makes me think I am missing something here.
There was a problem hiding this comment.
No, this is just me. Sorry.
There was a problem hiding this comment.
I actually believe that's an error in the original file. I should not state what type of pixel is being set, and the capitalisation is wrong. But I'll fix that later.
fb/xrgb_test.go
Outdated
| t.Fatalf("failed to read src image file %v.png: %v", test, err) | ||
| } | ||
|
|
||
| got := NewRGB565(src.Bounds()) |
fb/xrgb_test.go
Outdated
| if err != nil { | ||
| t.Fatalf("failed to read golden image file %v-xrgb.png: %v", test, err) | ||
| } | ||
| want := NewRGB565(gol.Bounds()) |
| @@ -0,0 +1,109 @@ | |||
| // Copyright ©2016 The ev3go Authors. All rights reserved. | |||
There was a problem hiding this comment.
As attested by the horrific level of copy-paste errorism, the code was originally the 565 code.
fb/xrgb.go
Outdated
| // NewXRGB returns a new XRGB image with the given bounds. | ||
| func NewXRGB(r image.Rectangle) *XRGB { | ||
| w, h := r.Dx(), r.Dy() | ||
| stride := 2 * w |
fb/xrgb.go
Outdated
| // Pix holds the image's pixels, as 32 bit XRGB | ||
| // values stored in little-endian order. | ||
| // The pixel at (x, y) is the four bytes at | ||
| // Pix[2*(x-Rect.Min.X) + (y-Rect.Min.Y)*Stride]. |
|
Ah, sorry, I had the comments ready for some time and now I submitted them without checking the current state of discussion. |
|
Sorry for that. I should not code just before bed. The issues were largely copy-paste errors. The issue about the return type is a Go-ism for interface satisfaction. |
|
The operations on the colour and alpha channels in the color model are because the |
|
Updates ev3go/ev3#2. |
@dlech Please take a look.
This will provide the construction function required for ev3go/ev3#3.