Opened 12 years ago

Closed 12 years ago

Last modified 2 years ago

#8934 closed patch

GRAPHICS: PixelFormat introduction

Reported by: lordhoto Owned by: lordhoto
Priority: normal Component: Graphics
Keywords: Cc:



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)

pixelformat_v1.patch (8.3 KB ) - added by lordhoto 12 years ago.
Patch against r34871.

Download all attachments as: .zip

Change History (5)

by lordhoto, 12 years ago

Attachment: pixelformat_v1.patch added

Patch against r34871.

comment:1 by fingolfin, 12 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 lordhoto, 12 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 lordhoto, 12 years ago

Owner: changed from fingolfin to lordhoto
Status: newclosed

comment:4 by digitall, 2 years ago

Component: Graphics
Note: See TracTickets for help on using tickets.