[Scummvm-devel] Bitdepth/pixel format API concerns

[J Northup]

On Fri, Jun 12, 2009 at 6:29 PM, Johannes Schickel <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>> 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.
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?


> > 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...

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.


> > The compromise that LordHoto and I came to on IRC is thus:
> > Engines use the colormode enum type, and pass a list of supported
> > colormodes to initGraphics.
>
>
> Yes.
>
> > 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...

> 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...


> > 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.

> 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.
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.
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.

If you have any questions or complaints about the proposal, please reply.
If I do not recieve any complaints about this proposal, I will post it and a
list of other proposals currently under consideration on my blog to solicit
further feedback about which option is best.

I sincerely apologize if I seem stubborn about this. What is happening is
that each option continues to evolve in absence of discussion, so that by
the time people say "we should do this", I have a set of better proposals. I
will be more transparent about any further refinements to these proposals,
adding the information to the discussion as soon as it becomes available,
because I really do not want to tie the process of finalizing the API up any
further than it already has been.

Jody
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20090612/8c5c4ab4/attachment.html>