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

Added support for multicolor pixmaps in drawRGBBitmap. #39

Merged
merged 3 commits into from May 5, 2017

Conversation

Projects
None yet
3 participants
@marcmerlin
Contributor

marcmerlin commented Jan 4, 2015

This adds support for multicolor bitmaps as supported by my software PWM matrix driver

snap1
snap2
snap3

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
Contributor

marcmerlin commented Jan 5, 2015

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Nov 11, 2015

Contributor

I just had one of my users confused because he was using the wrong GFX library (yours, not mine) and I just realized that I still have a fork from you because of this patch.
Since this is a generic function for your generic GFX library, would you be willing to add it?

Contributor

marcmerlin commented Nov 11, 2015

I just had one of my users confused because he was using the wrong GFX library (yours, not mine) and I just realized that I still have a fork from you because of this patch.
Since this is a generic function for your generic GFX library, would you be willing to add it?

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Apr 6, 2017

Contributor

As a followup, I have tested this code with adafruit_neomatrix, and it works out of the box.

Contributor

marcmerlin commented Apr 6, 2017

As a followup, I have tested this code with adafruit_neomatrix, and it works out of the box.

@PaintYourDragon

This comment has been minimized.

Show comment
Hide comment
@PaintYourDragon

PaintYourDragon Apr 6, 2017

Member

Just heading out for a weekend thing but will have a good look and likely merge this when I get back. I've been hesitant to add 565-specific functions to core GFX, but after some things went not-so-well in one of the subclasses, this might just be the way to go. Tangent ideas are brewing, this'll be a good starting point. Thanks!

Member

PaintYourDragon commented Apr 6, 2017

Just heading out for a weekend thing but will have a good look and likely merge this when I get back. I've been hesitant to add 565-specific functions to core GFX, but after some things went not-so-well in one of the subclasses, this might just be the way to go. Tangent ideas are brewing, this'll be a good starting point. Thanks!

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Apr 6, 2017

Contributor

@PaintYourDragon thanks for the reply. For what it's worth, this code is not 565 specific at all. I wrote it for my own backend that happens to be 444 BGR and then used it unmodified on 565 RGB neomatrix and it just worked for both because it passes the 16 bits of colors unmodified without caring how they are interpreted by the backend after that.
I agree that it won't work for 24bit color, but my understanding is that adafruit-gfx doesn't really deal well with that anyway, and that 24bit color is kind of overrated for many displays anyway as one of your pages explains (i.e. you can't tell the difference from more bits)
Enjoy your weekend :)

Contributor

marcmerlin commented Apr 6, 2017

@PaintYourDragon thanks for the reply. For what it's worth, this code is not 565 specific at all. I wrote it for my own backend that happens to be 444 BGR and then used it unmodified on 565 RGB neomatrix and it just worked for both because it passes the 16 bits of colors unmodified without caring how they are interpreted by the backend after that.
I agree that it won't work for 24bit color, but my understanding is that adafruit-gfx doesn't really deal well with that anyway, and that 24bit color is kind of overrated for many displays anyway as one of your pages explains (i.e. you can't tell the difference from more bits)
Enjoy your weekend :)

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Apr 6, 2017

Contributor

During testing, I found it helpful to be able to display bitmaps that have been generated or modified in RAM, so I made a small change to allow this.
I've tested it in 328p with NeoMatrix and it works fine with and without progmem.

Contributor

marcmerlin commented Apr 6, 2017

During testing, I found it helpful to be able to display bitmaps that have been generated or modified in RAM, so I made a small change to allow this.
I've tested it in 328p with NeoMatrix and it works fine with and without progmem.

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin
Contributor

marcmerlin commented Apr 6, 2017

@pacav69

This comment has been minimized.

Show comment
Hide comment
@pacav69

pacav69 Apr 8, 2017

this issue was raised over 2 years ago i think it would be prudent to add this minor change to this wonderful library.

pacav69 commented Apr 8, 2017

this issue was raised over 2 years ago i think it would be prudent to add this minor change to this wonderful library.

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Apr 9, 2017

Contributor

@pacav69 . He already replied that he was going to review it soon. Please give him a chance to :)

Contributor

marcmerlin commented Apr 9, 2017

@pacav69 . He already replied that he was going to review it soon. Please give him a chance to :)

@pacav69

This comment has been minimized.

Show comment
Hide comment
@pacav69

pacav69 Apr 9, 2017

@marcmerlin i know i just thought i would give him a nudge and give you a helping hand as it is most annoying that i have to use a different version so that i can compile your code.

pacav69 commented Apr 9, 2017

@marcmerlin i know i just thought i would give him a nudge and give you a helping hand as it is most annoying that i have to use a different version so that i can compile your code.

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin Apr 25, 2017

Contributor

Hi @PaintYourDragon I now have 3 different demos on 3 different backends that depend on this patch. It is nice to be able to display multi color pixmaps on totally different backends that happen to support Adafruit::GFX even though they don't even have the same color bit depth.
Obviously, it would be very nice to have it integrated, more things are depending on it.

image

Contributor

marcmerlin commented Apr 25, 2017

Hi @PaintYourDragon I now have 3 different demos on 3 different backends that depend on this patch. It is nice to be able to display multi color pixmaps on totally different backends that happen to support Adafruit::GFX even though they don't even have the same color bit depth.
Obviously, it would be very nice to have it integrated, more things are depending on it.

image

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin May 3, 2017

Contributor

@PaintYourDragon
Ok, this code has now been tested on 328p, Teensy 3.1 and ESP8266 with 3 different Adafruit GFX backends that kind of rely on this now, for their demos: my own ( https://github.com/marcmerlin/LED-Matrix/ ), NeoMatrix, and RGB-matrix-Panel
Honestly, not being able to display RGB pixmaps on those backends is pretty limiting and this patch fixes this with very little code.
Could you please merge, and this will then also unblock the other demos I've submitted (which you are welcome not to merge, I'm just giving those out for people who care for better demos/examples)

Contributor

marcmerlin commented May 3, 2017

@PaintYourDragon
Ok, this code has now been tested on 328p, Teensy 3.1 and ESP8266 with 3 different Adafruit GFX backends that kind of rely on this now, for their demos: my own ( https://github.com/marcmerlin/LED-Matrix/ ), NeoMatrix, and RGB-matrix-Panel
Honestly, not being able to display RGB pixmaps on those backends is pretty limiting and this patch fixes this with very little code.
Could you please merge, and this will then also unblock the other demos I've submitted (which you are welcome not to merge, I'm just giving those out for people who care for better demos/examples)

@PaintYourDragon PaintYourDragon merged commit 4b1a8a6 into adafruit:master May 5, 2017

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin May 5, 2017

Contributor

Thanks a bunch.

Contributor

marcmerlin commented May 5, 2017

Thanks a bunch.

@marcmerlin marcmerlin deleted the marcmerlin:drawrgb branch May 5, 2017

@PaintYourDragon

This comment has been minimized.

Show comment
Hide comment
@PaintYourDragon

PaintYourDragon May 5, 2017

Member

Thanks for your patience, was handling a variety of other things.

Minor detail, I might go in and tweak this to have separate const (PROGMEM) and non-const (RAM) versions, consistent with the existing drawBitmap() functions, rather than the extra parameter.

Member

PaintYourDragon commented May 5, 2017

Thanks for your patience, was handling a variety of other things.

Minor detail, I might go in and tweak this to have separate const (PROGMEM) and non-const (RAM) versions, consistent with the existing drawBitmap() functions, rather than the extra parameter.

@marcmerlin

This comment has been minimized.

Show comment
Hide comment
@marcmerlin

marcmerlin May 5, 2017

Contributor

Understood. If you do so, I'll modify the code that calls into it and send updated demo patches, no problem at all.

Contributor

marcmerlin commented May 5, 2017

Understood. If you do so, I'll modify the code that calls into it and send updated demo patches, no problem at all.

@PaintYourDragon

This comment has been minimized.

Show comment
Hide comment
@PaintYourDragon

PaintYourDragon May 6, 2017

Member

OK yeah, for consistency with other bitmap-related functions I've broken it out into PROGMEM- and RAM-resident versions of the function (instead of a trailing progmem argument).

Also added a variant that lets you additionally pass a bitmask to make non-rectangular overlays.

Member

PaintYourDragon commented May 6, 2017

OK yeah, for consistency with other bitmap-related functions I've broken it out into PROGMEM- and RAM-resident versions of the function (instead of a trailing progmem argument).

Also added a variant that lets you additionally pass a bitmask to make non-rectangular overlays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment