#8934 closed patch
GRAPHICS: PixelFormat introduction
| Reported by: | lordhoto | Owned by: | lordhoto |
|---|---|---|---|
| Priority: | normal | Component: | Graphics |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Hi,
since we need some way to get a usable pixel format description of the overlay's one, I started to create a SDL_PixelFormat similar struct: Graphics::PixelFormat. This patch basically only adds PixelFormat and a few helpers (RGBToColor, ARGBToColor, colorToRGB, colorToARGB).
The helper functions are currently marked as 'inline', one might want to think about that, but I think it should be fine.
The patch also adds a (documented) method "getOverlayFormat" to OSystem. SDL backend is updated to support it.
Usage for this struct might be: - GUI's VectorRenderer - Scalers, seeing patch tracker item #1664514 "Support for a 32 bit _hwscreen in the SDL backend" - Graphics::Surface to connect a given pixel format with an surface -> GUI's GraphicsWidget, to allow easier conversion to overlay format (falls under Graphics::Surface connection of course)
Ticket imported from: #2216641. Ticket imported from: patches/1039.
Attachments (1)
Change History (5)
by , 16 years ago
| Attachment: | pixelformat_v1.patch added |
|---|
comment:1 by , 16 years ago
Go ahead and commit this. My thoughts:
* We should make virtual Graphics::PixelFormat getOverlayFormat() const = 0; a mandatory OSystem API. And add implementions for it to all ports where it's obvious what the correct pixelformat is. And then notify all porters via scummvm-devel about this and how they can adapt/fix their port.
* Do we really need uint32 rMask, gMask, bMask, aMask; ? That's 12 bytes that seem redundant. The only possible I can imagine is if not all bits of a color are adjacent; but I don't see how the code in the patch (equivalent to the code in SDL) would support that. Maybe I am missing something here? Please enlighten me... Otherwise, my proposal is to for example replace (((r >> fmt.rLoss) << fmt.rShift) & fmt.rMask) by (((r >> fmt.rLoss) << fmt.rShift)) and also replace r = ((color & fmt.rMask) >> fmt.rShift) << fmt.rLoss; by r = ((color >> fmt.rShift) << fmt.rLoss) & 0xFF;
* Next, I would like Graphics::Surface to becomes fully pixelformat ready -- by replacing uint8 bytesPerPixel; with a PixelFormat pointer / member. Note: This would require all OSystem::lockScreen implementations to be adapted. And of course all client code using a PixelFormat...
comment:2 by , 16 years ago
Well actually OSystem::getOverlayFormat should really be implemented by all ports, since after all it's a pure virtual function and there's no comment about it being optional or something.
I'll write an to -devel about that soon.
As I stated in the tracker description I'm all for that Graphics::Surface change, but after all for 1 byte per pixel (aka paletted images), we might want to think whether we (optionally?) want to connect a palette to it or not.
I'll commit the patch now without rMask, gMask, bMask and aMask as suggested.
comment:3 by , 16 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → closed |
comment:4 by , 6 years ago
| Component: | → Graphics |
|---|
Patch against r34871.