-
Notifications
You must be signed in to change notification settings - Fork 197
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
pixel: add NewImageFromBytes function #713
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,40 @@ func NewImage[T Color](width, height int) Image[T] { | |
} | ||
} | ||
|
||
// NewImageFromBytes creates a new image of the given size using an existing data slice of bytes. | ||
func NewImageFromBytes[T Color](width, height int, buf []byte) Image[T] { | ||
if width < 0 || height < 0 || int(int16(width)) != width || int(int16(height)) != height { | ||
// The width/height are stored as 16-bit integers and should never be | ||
// negative. | ||
panic("NewImageFromBytes: width/height out of bounds") | ||
} | ||
var zeroColor T | ||
var data unsafe.Pointer | ||
switch { | ||
case zeroColor.BitsPerPixel()%8 == 0: | ||
// Typical formats like RGB888 and RGB565. | ||
// Each color starts at a whole byte offset from the start. | ||
if len(buf) != width*height*int(unsafe.Sizeof(zeroColor)) { | ||
panic("NewImageFromBytes: data slice size mismatch") | ||
} | ||
data = unsafe.Pointer(&buf[0]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't safe. If you create this from a plain old byte slice ( This applies to RGB565BE and RGB555 which have an underlying type of To check for this, you can do something like this: if zeroColor.BitsPerPixel() % 8 == 0 && uintptr(data) % unsafe.Alignof(zeroColor) != 0 {
panic("NewImageFromBytes: byte buffer is not aligned")
} But really, if you allocate the slice using any normal Go way (global variable, if zeroColor.BitsPerPixel() % 8 == 0 && unsafe.Alignof(zeroColor) > 1 {
panic("NewImageFromBytes: cannot convert byte buffer to image (different alignment)")
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will add a new PR with that additional check, since this code is specifically for mono. |
||
default: | ||
// Formats like RGB444 that have 12 bits per pixel. | ||
// We access these as bytes, so allocate the buffer as a byte slice. | ||
bufBits := width * height * zeroColor.BitsPerPixel() | ||
bufBytes := (bufBits + 7) / 8 | ||
if len(buf) != bufBytes { | ||
panic("NewImageFromBytes: data slice size mismatch") | ||
} | ||
data = unsafe.Pointer(&buf[0]) | ||
} | ||
return Image[T]{ | ||
width: int16(width), | ||
height: int16(height), | ||
data: data, | ||
} | ||
} | ||
|
||
// Rescale returns a new Image buffer based on the img buffer. | ||
// The contents is undefined after the Rescale operation, and any modification | ||
// to the returned image will overwrite the underlying image buffer in undefined | ||
|
@@ -105,14 +139,15 @@ func (img Image[T]) setPixel(index int, c T) { | |
case zeroColor.BitsPerPixel() == 1: | ||
// Monochrome. | ||
x := index % int(img.width) | ||
y := index / int(img.width) | ||
offset := x + (y/8)*int(img.width) | ||
offset := index / 8 | ||
Comment on lines
-108
to
+142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are also changing the format here! The offset calculation changed and now outputs different values. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above comment. |
||
|
||
ptr := (*byte)(unsafe.Add(img.data, offset)) | ||
if c != zeroColor { | ||
*((*byte)(ptr)) |= 1 << uint8(y%8) | ||
*((*byte)(ptr)) |= (1 << (7 - uint8(x%8))) | ||
} else { | ||
*((*byte)(ptr)) &^= 1 << uint8(y%8) | ||
*((*byte)(ptr)) &^= (1 << (7 - uint8(x%8))) | ||
Comment on lines
-112
to
+148
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are changing the format here! Specifically, this reverses the order of the bits in the If you want to add a new format, please do so by adding a new pixel type instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I was the only one trying to use this format, and I did not complete that work (on Thumby) due to this code being incorrect in the current form. So I am pretty sure this is fixing something broken, not introducing a new format. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, seems reasonable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good suggestion, I will do that in the same PR in which I add the alignment check. |
||
} | ||
|
||
return | ||
case zeroColor.BitsPerPixel()%8 == 0: | ||
// Each color starts at a whole byte offset. | ||
|
@@ -166,9 +201,10 @@ func (img Image[T]) Get(x, y int) T { | |
case zeroColor.BitsPerPixel() == 1: | ||
// Monochrome. | ||
var c Monochrome | ||
offset := x + (y/8)*int(img.width) | ||
offset := index / 8 | ||
bits := index - (offset * 8) | ||
ptr := (*byte)(unsafe.Add(img.data, offset)) | ||
c = (*ptr >> uint8(y%8) & 0x1) == 1 | ||
c = ((*ptr >> (7 - uint8(bits))) & 0x1) > 0 | ||
return any(c).(T) | ||
case zeroColor.BitsPerPixel()%8 == 0: | ||
// Colors like RGB565, RGB888, etc. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this return
(Image[T], error)
instead of panicking?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the current interface, which I was not trying to change in this PR.