Skip to content

Commit

Permalink
fix wrong order if p_bitmap->depth > 16 in UpdatePinmameDisplayBitmap
Browse files Browse the repository at this point in the history
  • Loading branch information
@toxieainc
toxieainc committed Jan 5, 2024
1 parent 6bb14f6 commit d0528cb
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions src/libpinmame/libpinmame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,14 +217,14 @@ int GetDisplayCount(const struct core_dispLayout* p_layout, int* const p_index)

int UpdatePinmameDisplayBitmap(PinmameDisplay* pDisplay, const struct mame_bitmap* p_bitmap)
{
UINT8 r,g,b;
UINT8* dst = (UINT8*)pDisplay->pData;
UINT8* __restrict dst = (UINT8*)pDisplay->pData;
int diff = 0;

if (p_bitmap->depth == 8) {
for(int j = 0; j < pDisplay->layout.height; j++) {
UINT8* src = (UINT8*)p_bitmap->line[j];
const UINT8* __restrict src = (UINT8*)p_bitmap->line[j];
for(int i=0; i < pDisplay->layout.width; i++) {
UINT8 r,g,b;
palette_get_color((*src++),&r,&g,&b);
if (dst[0] != r || dst[1] != g || dst[2] != b)
diff = 1;
Expand All @@ -236,8 +236,9 @@ int UpdatePinmameDisplayBitmap(PinmameDisplay* pDisplay, const struct mame_bitma
}
else if(p_bitmap->depth == 15 || p_bitmap->depth == 16) {
for(int j = 0; j < pDisplay->layout.height; j++) {
UINT16* src = (UINT16*)p_bitmap->line[j];
const UINT16* __restrict src = (UINT16*)p_bitmap->line[j];
for(int i=0; i < pDisplay->layout.width; i++) {
UINT8 r,g,b;
palette_get_color((*src++),&r,&g,&b);
if (dst[0] != r || dst[1] != g || dst[2] != b)
diff = 1;
Expand All @@ -249,11 +250,12 @@ int UpdatePinmameDisplayBitmap(PinmameDisplay* pDisplay, const struct mame_bitma
}
else {
for(int j = 0; j < pDisplay->layout.height; j++) {
UINT32* src = (UINT32*)p_bitmap->line[j];
const UINT32* __restrict src = (UINT32*)p_bitmap->line[j];
for(int i=0; i < pDisplay->layout.width; i++) {
UINT8 r,g,b;
palette_get_color((*src++),&r,&g,&b);
if (dst[0] != r || dst[1] != g || dst[2] != b)
diff = 1;
palette_get_color((*src++),&r,&g,&b);
*(dst++) = r;
*(dst++) = g;
*(dst++) = b;
Expand Down

2 comments on commit d0528cb

@jsm174
Copy link
Contributor

@jsm174 jsm174 commented on d0528cb Jan 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@toxieainc, thanks!

quick question, I thought I read somewhere, it's better to create reused vars outside of for loops (ie r, g, b), so they don't get created each iteration.

			for(int i=0; i < pDisplay->layout.width; i++) {
				UINT8 r,g,b;

vs

			UINT8 r,g,b;
			for(int i=0; i < pDisplay->layout.width; i++) {

If that's not true, I'll redo my coding style.

@toxieainc
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends.. :)

My rule of thumb is, that everything should be declared as close to its use-location as possible, unless the creation of a var is very costly and/or the constructor is basically run unnecessarily (e.g. a vector that could be reused multiple times (loop) with roughly the same max size, or a large struct that is re-initialized with the same values again and again).

So for simple data types like ints, floats, etc, its usually always better to have them within a very limited scope. This makes live for the compiler (and most of the time also the human reader) easier.
While one can argue that the compiler is smart enough to track liveness, etc, my professional experience with compilers is, that they work counter-intuitive (compared to human logic) in many cases, so its better to be safe than sorry IMHO.

Its also what static analyzers recommend nowadays for simple data types.

Please sign in to comment.