Opened 11 years ago

Closed 7 years ago

Last modified 7 months ago

#8932 closed patch (wontfix)

Gamma support

Reported by: bluegr Owned by:
Priority: normal Component: Graphics
Keywords: Cc:
Game:

Description

This is a patch that adds gamma support and a gamma slider to ScummVM's options (part of feature request #2116244). It does this by calling SDL_SetGamma, so it's done in hardware level, and it doesn't affect any engine

It's working, though there are still some unfinished things:
- The gamma value is not set yet when ScummVM is loaded
- Currently, the gamma values range from 0-200 (or 0.0 to 2.0). I'm not sure about the actual maximum value, though 2.0 seems to be very bright
- When the options dialog pops up, the initial value of the slider is not set correctly
- I'm not sure if the graphics tab is a good place to put the slider, as it contains per-game options. Perhaps the misc tab would be more appropriate?
- I haven't documented functions setGamma() and resetGamma() in system.h
- SDL_SetGamma() accepts a float, which is what setGamma() accepts. I'm not sure if there should be any floats in system.h, in case it's used by systems where floats are not supported (one of the reasons I haven't documented the function). Perhaps it would be wiser to change the float parameter to an integer (which would be the current parameter multiplied by 100)

Also, note that the XML theme files contain the save/load buttons from the common load dialog patch, which are not needed in this patch, I just haven't removed them (and I was hesitant to edit the actual patch file itself, in case I break it)

Ticket imported from: #2201951. Ticket imported from: patches/1037.

Attachments (1)

gamma.patch (8.0 KB) - added by bluegr 11 years ago.
ScummVM gamma support

Download all attachments as: .zip

Change History (6)

Changed 11 years ago by bluegr

Attachment: gamma.patch added

ScummVM gamma support

comment:1 Changed 11 years ago by lordhoto

Looks quite fine to me, but I would really strip out the bits from common save/load dialog.

For the dialog issue, couldn't we just setup gamma values per default to 1(.)00? Then it should be easy to query the values from ConfMan and setup the sliders accordingly.

On startup it should be trivial to add an setGamma call with the values from ConfMan, so it is set up properly.
Places for that would be: base/main.cpp l70 for launcher and base/main.cpp around l209 for game startup.

Actually one might really think of using integer values from 0 to 200 for the gamma configuration, first of all we don't support saving/loading floats via ConfMan currently. Next the values in the config file would match exactly the values in the options dialog.

comment:2 Changed 11 years ago by fingolfin

* This is a highly port specific change. As such, I feel quite uncomfortable about adding an OSystem API for this, and even less about adding this to the generic options dialog. If this was added to a port specific tab of the options dialog, which would be handled by port specific code, and thus would not require extending the OSystem API, I'd approve this patch instantly. But as it is, I am quite reluctant. (Esp. since I personally fail to see the usefulness of it).

* Using a float/double for a config param is a big no-no, use a scaled integer instead, as you suggested yourself. For consistency, the internal API then should use an integer range, too, that way, the conversions (with the impiöed accuracy loss) are reduced to a minimum

comment:3 Changed 10 years ago by sev-

I propose to reject this patch then.

comment:4 Changed 7 years ago by fuzzie

Resolution: wontfix
Status: newclosed

comment:5 Changed 7 months ago by digitall

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