[Scummvm-devel] Bitdepth/pixel format API concerns

Johannes Schickel lordhoto at gmail.com
Sat Jun 13 16:23:06 CEST 2009


J Northup wrote:
> On Fri, Jun 12, 2009 at 6:29 PM, Johannes Schickel <lordhoto at gmail.com 
> <mailto:lordhoto at gmail.com>> wrote:
>
>     J Northup wrote:
>     >
>     >
>     > On Thu, Jun 11, 2009 at 7:08 AM, Max Horn <max at quendi.de
>     <mailto:max at quendi.de>
>     > <mailto:max at quendi.de <mailto:max at quendi.de>>> wrote:
>     >
>     >
>     >     So far, I see no actual cons to using Graphics::PixelFormat -- I
>     >     think LordHoto refuted all claims otherwise so far. Maybe we
>     miss
>     >     something, though -- then I'd like to hear concrete arguments,
>     >     though ;).
>     >
>     >
>     >     Bye,
>     >     Max
>     >
>     >
>     >
>     > Sigh, I should have made a posting to the list right after the
>     > conversation LordHoto and I had on IRC wednesday night.
>     > The cons to using Graphics::PixelFormat are
>     > 1) It is overkill -- it easily allows engines to specify a
>     format that
>     > is completely impossible, by mistake
>
>     And the engine author would easily notice it, because his screen would
>     look strange. So? It's like that they have a typo in the enum...
>
>     > 2) It requires the engine to specify color order, meaning that a
>     minor
>     > oversight on the part of the engine developer could result in it
>     being
>     > unusable on all backends with a different color order.
>
>     Why? Your idea was anyway to let the backend worry about such
>     conversions, thus it would work just fine. Maybe explain this to
>     me again.
>
>
> Err, that was my original idea several weeks ago. You convinced me 
> that it wouldn't be a good way to handle it.

Well I was convinced by Max and by Kirben that we want backends to do 
the conversion. Since Kirben said doing conversion in SCUMM would be 
pretty much a horror, I think we should let backends worry about it.

> My current idea is:
> Backends which have hardware support for format conversion always 
> respond that formats are supported, and handle conversion.
> Backends which would be slowed by conversion respond that they don't 
> support formats that they can't display natively.
> Engines whose preferred format is not supported either convert pixel 
> formats on image loading (using generic functions provided for this 
> purpose), or error out and refuse to run entirely.
>  
>
>     > 3) Additionally, the wide range of color orders that the engine
>     could
>     > potentially request necessitate a great deal of sanity checking
>     on the
>     > part of engines that only support one.
>
>     I still don't get this one, sorry.
>
> Do you understand what sanity checking is?
> The backend would have to run a variety of tests to make sure that 
> colors are in the correct order (eg: RGBA), or the PixelFormat won't 
> make any sense to them. If backends aren't going to use the color 
> order specified by the PixelFormat, then why use a PixelFormat at all?

This one was obviously caused by the fact, that you thought the backend 
shouldn't do conversion. Since if it does conversion with every mode, 
there's no need for sanity checks, it should just use it to convert it 
to its native format.

>  
>
>     > 4) Setting up a Graphics::PixelFormat structure takes a minimum of 8
>     > lines of code per structure. Engines like gob and groovie which
>     > convert from YUV have to set up a list of several, and the
>     difference
>     > in coding required adds up over the course of three current and an
>     > unknown number of future supported engines.
>
>     GOB and Groovie won't have to setup any PixelFormat struct.
>     They'll just
>     use the backend function to query all native pixel formats, you didn't
>     implement that yet though :-).
>
> You keep saying that, but nobody else agreed with that...

DrMcCoy, the GOB engine author, did in IRC.

[20:01] <LordHoto> also it might be nice to have some function to query 
supported pixel formats
[20:02] <LordHoto> since for example gob, which only does YUV->RGB 
conversion AFAIK, would probably be able to work with *all* modes out there
[20:02] <LordHoto> so if the backend says: "hey I'm supporting A8R8G8B8" 
it can work with it the same way as with "hey I'm supporting B8G8R8A8" 
or "R8G8B8A8"
[20:14] <DrMcCoy> LordHoto: Yeah, that's what I had in mind anyway, 
query for something and then grab the modes with the highest colors and 
out of these the one with the "fastes" conversion
[20:15] <DrMcCoy> Errr, query for the modes*

Source: 
http://logs.scummvm.org/log.php?log=scummvm.log.10Jun2009&format=html


>
>     With the SDL backend this isn't really possible, since you can
>     first be
>     sure to see the mode it chooses, when you setup a real video mode.
>     So we
>     might need to think about that. Maybe we'll just return that no pixel
>     format can be used natively. Thus the engine just uses the best
>     mode it
>     supports and let the SDL backend worry about conversions. This might
>     need some more thought!
>
> I think you aren't understanding this. The idea isn't that the 
> compatibility list is a list of natively supported formats.
> The idea is that the compatibility list is a list of formats that the 
> backend can accept from the engine. If the backend responds that the 
> format is unsupported, then the engine <b>can not</b> use that format 
> for data.

I guess you didn't understand my point. It's probably because we had 
different ideas of that. I thought about a natively supported 
PixelFormat list, while you thought about a hardware conversion 
PixelFormat list.

>     > initGraphics gets the color order (RGBA, ABGR) from the backend, and
>     > converts the colormode enums.
>
>     No the color order has to be fixed, if the engine requests RGB555, it
>     has to be R, G, B order. An engine not doing any conversions can't
>     easily switch between RGBA and ABGR, the screen would look wrong.
>
> That's why I said that the engine <b>must</b> do conversions...

Well the backend can do that too, so there's no real must. And it seemed 
to me like it's common consensus, that we require the backends to do that.

>
>     > into Graphics::PixelFormat structures using a generic function
>     > provided for this purpose.
>     > initGraphics queries the backend regarding format compatibility, and
>     > passes the first compatible format to the backend for initialization
>     > during the gfx transaction.
>
>     Yes. (for cases, there's no natively supported format, check the next
>     answer).
>
>     > initGraphics errors out if endGfxTransaction returns an error in
>     > setting up the screen with requested pixel format
>     > engines query backend to determine the accepted pixel format
>
>     No, if there's no native supported format, the backend should do
>     conversions. I thought we did agree on that.
>
> You were the one who convinced me that this was a bad idea...

Yes and Max voted for it and since Kirben said SCUMM would need to rely 
on this, I changed my position.

>  
>
>     > When loading graphical resources, engines call a generic color-order
>     > swapping function as necessary.
>
>     Kirben told me that those conversions would make the SCUMM code much
>     more complex, but I'm not exactly sure. Kirben, maybe you can
>     comment on
>     this again? Anyway since the backend is forced to do conversion, this
>     isn't exactly needed, it might be used by engine authors when they
>     expire performance problems on certain backends. (See the mail
>     from Max
>     on 9th of July 10:54 AM).
>
> I am very confused as to why you believe we agreed that the backend is 
> forced to do conversions. I said this at first, but you said it was a 
> bad idea, and after a bit of discussion, I agreed with you that it was 
> a bad idea.

Well I thought you wanted it, Max wanted it, Kirben said it would be 
best for SCUMM. So I thought we should really force the backend to do that.

>
>     > This way, engines get the simplicity of the enum type, but
>     backends do
>     > not have to worry about dealing with any format but
>     Graphics::PixelFormat.
>     > I am working on the proof of concept for this, but if there are any
>     > complaints about this remaining, I will go ahead with one that uses
>     > Graphics::PixelFormat exclusively.
>
>     Just for completeness, here's how I think it should work:
>
>     The backend is required to implement the following functionality:
>
>     "initFormat", used for initializing a specific format. This takes a
>     PixelFormat.
>     "queryNativeFormats", this returns a list of PixelFormat structs
>     supported without conversion by the backend.
>     "getScreenFormat", used to return the currently setup PixelFormat.
>
>     Now there are different possibilites:
>
>     1) The engine can work with any mode out there:
>
>     In this case the engine should just use "queryNativeFormats" and
>     select
>     the one it thinks would be best. If there's no natively supported
>     format, let it setup *any* format and let the backend worry how to
>     deal
>     with it.
>
>     ALTERNATIVE proposal: let OSystem::initFormat take a pointer to a
>     PixelFormat, if that one is zero, let the backend choose the mode it
>     thinks is best for it. The engines will then just use the format
>     returned by "getScreenFormat".
>
>     That alternative proposal might be best for the SDL backend, since
>     it'll
>     first know the native mode when it has the video mode setup via
>     SDL_SetVideoMode.
>
>     2) The engine has a limited set of formats (at least 2):
>
>     Let the engine pass a list of the enums to "initGraphics", as you
>     proposed. "initGraphics" is now supposed to use the first format
>     supported natively by the backend. If not it errors out. (One could
>     think of some default graphics mode parameter, which will be used then
>     no format has native supported, so the backend does conversion).
>
>     All this is implemented by comparing requested formats to the return\
>     value of "OSystem::queryNativeFormats".
>
>     3) The engine can only work with a single format:
>
>     Pass that mode directly to initFormat and let the backend worry about
>     conversions to it's native mode.
>
>     // Johannes
>
>
> Okay, let me try to explain my proposal better, because there has 
> definitely been a misunderstanding.
>
> The backend is required to implement:
> void initFormat(Graphics::PixelFormat format): used for setting up the 
> backend with a requested format.
> bool isABGR(void): true if the backend uses ABGR color order, false if 
> backend uses RGBA.

The return value of getScreenFormat can be used for this one, so I don't 
see the real need for that. And again this should just return the color 
order setup by initFormat, so this seems even more confusing to me.

> bool isFormatCompatible(Graphics::PixelFormat format): true if the 
> backend can accept data in that format, false if it can't
> Graphics::PixelFormat getScreenFormat(void): returns the currently set 
> pixel format.
>
> Now, there are two possible cases for the backend:
> 1) Backends which support fast conversion from any format, which will 
> always return true from isFormatCompatible.

A side note: as I get the SDL documentation there is no way of telling 
for which modes this is ture for SDL, thus it should only return CLUT8 
as supported with your option, right?

> 2) Backends which support a limited number formats, which will 
> fallback to CLUT8 if no compatible format is listed.
>
> With backend case 2, there are two possible cases for the engine
> 1) Engine has an 8-bit fallback mode
> 2) Engine reports an error and returns immediately.
>
> Engines, meanwhile, are required to support ABGR and RBGA color modes, 
> via convenience functions uint32 toABGR(uint32 color) and uint32 
> toRBGA(uint32 color) which I will provide.

Two PixelFormat structs can be used for conversion here (even for 
arbitrary conversion, not just RGBA->ABGR). One the engine provides and 
the one from OSystem::getScreenFormat. With that the engine would be 
able to do every conversion itself. If the engine is able to do the 
swapping it's anyway probably able to do any conversion by itself.

As Kirben explained this is hardly possible with SCUMM (at least without 
much more complexity as I got it), so I don't think this will work.

>
> If you have any questions or complaints about the proposal, please reply.

I still think mine is better. :-)

For example yours has no way to query the native supported formats, 
which is the best you can get if the engine can do a one time conversion 
on load or does YUV->RGB conversion. (RGB doesn't imply the color order 
here). Mine has that.

Mine still has the possibility to setup a fast mode for the backend. In 
the end in my proposal backends with fast hardware conversion could just 
return the modes via "queryNativeFormats" too, so engines with a limited 
set of modes could take the same advantage as with yours. (Maybe we 
could name it "queryFastFormats" then).

Backends known to have major performance troubles can just not include 
the specific engines/games relying on that in their builds till there's 
support for fast conversions in the engine/the backend. So the whole 
"fail if no format supported" or performance issues can be hidden from 
the user.

// Johannes




More information about the Scummvm-devel mailing list