Skip to content

Commit

Permalink
Fixed bug 4938 - Suggestion: rename SDL_PIXELFORMAT_BGR888 to SDL_PIX…
Browse files Browse the repository at this point in the history
…ELFORMAT_XBGR8888

Ellie

I just tripped over this: stb_image when requesting 3 channels with 8-bit actually returns them as 3 bytes per pixel with no alignment, so basically 4 pixels are 12 bytes with no padding (0...2, 3...5, 6...8, and 9...11). This I would have naively expected to be called RGB888 or BGR888, since there is no "dead" unused byte as I would expect for something called e.g. RGBX8888.

However, SDL2's SDL_PIXELFORMAT_BGR888 uses 4 bytes, same as SDL_PIXELFORMAT_BGRX8888, even though the latter appears to be a longer storage format - which it isn't, internally. It's just swapped, in byte order X, B, G, R (instead of BGRX). So why isn't the macro name also swapped, as "XBGR888" instead of just "BGR888"?

I find the formats therefore named inconsistently, and unless there is a reason for this I suggest these changes:

1. deprecate SDL_PIXELFORMAT_BGR888 in favor of a new SDL_PIXELFORMAT_XBGR8888

and

2. deprecate SDL_PIXELFORMAT_RGB888 in favor of a new SDL_PIXELFORMAT_XRGB8888
  • Loading branch information
slouken committed Jun 12, 2020
1 parent 2aa8974 commit 7c0986f
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions include/SDL_pixels.h
Expand Up @@ -188,18 +188,22 @@ typedef enum
SDL_PIXELFORMAT_RGB332 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED8, SDL_PACKEDORDER_XRGB,
SDL_PACKEDLAYOUT_332, 8, 1),
SDL_PIXELFORMAT_RGB444 =
SDL_PIXELFORMAT_XRGB4444 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED16, SDL_PACKEDORDER_XRGB,
SDL_PACKEDLAYOUT_4444, 12, 2),
SDL_PIXELFORMAT_BGR444 =
SDL_PIXELFORMAT_RGB444 = SDL_PIXELFORMAT_XRGB4444,
SDL_PIXELFORMAT_XBGR4444 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED16, SDL_PACKEDORDER_XBGR,
SDL_PACKEDLAYOUT_4444, 12, 2),
SDL_PIXELFORMAT_RGB555 =
SDL_PIXELFORMAT_BGR444 = SDL_PIXELFORMAT_XBGR4444,
SDL_PIXELFORMAT_XRGB1555 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED16, SDL_PACKEDORDER_XRGB,
SDL_PACKEDLAYOUT_1555, 15, 2),
SDL_PIXELFORMAT_BGR555 =
SDL_PIXELFORMAT_RGB555 = SDL_PIXELFORMAT_XRGB1555,
SDL_PIXELFORMAT_XBGR1555 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED16, SDL_PACKEDORDER_XBGR,
SDL_PACKEDLAYOUT_1555, 15, 2),
SDL_PIXELFORMAT_BGR555 = SDL_PIXELFORMAT_XBGR1555,
SDL_PIXELFORMAT_ARGB4444 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED16, SDL_PACKEDORDER_ARGB,
SDL_PACKEDLAYOUT_4444, 16, 2),
Expand Down Expand Up @@ -236,15 +240,17 @@ typedef enum
SDL_PIXELFORMAT_BGR24 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_ARRAYU8, SDL_ARRAYORDER_BGR, 0,
24, 3),
SDL_PIXELFORMAT_RGB888 =
SDL_PIXELFORMAT_XRGB8888 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED32, SDL_PACKEDORDER_XRGB,
SDL_PACKEDLAYOUT_8888, 24, 4),
SDL_PIXELFORMAT_RGB888 = SDL_PIXELFORMAT_XRGB8888,
SDL_PIXELFORMAT_RGBX8888 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED32, SDL_PACKEDORDER_RGBX,
SDL_PACKEDLAYOUT_8888, 24, 4),
SDL_PIXELFORMAT_BGR888 =
SDL_PIXELFORMAT_XBGR8888 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED32, SDL_PACKEDORDER_XBGR,
SDL_PACKEDLAYOUT_8888, 24, 4),
SDL_PIXELFORMAT_BGR888 = SDL_PIXELFORMAT_XBGR8888,
SDL_PIXELFORMAT_BGRX8888 =
SDL_DEFINE_PIXELFORMAT(SDL_PIXELTYPE_PACKED32, SDL_PACKEDORDER_BGRX,
SDL_PACKEDLAYOUT_8888, 24, 4),
Expand Down

0 comments on commit 7c0986f

Please sign in to comment.