Opened 10 years ago

Closed 10 years ago

Last modified 10 months ago

#9019 closed patch

GSOC: 16 bit game SDL surface

Reported by: SF/upthorn Owned by: sev-
Priority: normal Component: Graphics
Keywords: Cc:
Game:

Description

This adds a 16-bit surface to the SDL backend, for use by games... and modifies CopyRectToScreen et all to use it non-conditionally, to facilitate testing of the engine modules' support of 16-bit graphics.
Do NOT incorporate this into trunk as it will break display of all games.

This patch was built against revision 40366

Ticket imported from: #2794727. Ticket imported from: patches/1124.

Attachments (6)

16bit_step1.patch (5.4 KB ) - added by SF/upthorn 10 years ago.
The initial patch, unreviewed
16bit_step1_r1.patch (6.1 KB ) - added by SF/upthorn 10 years ago.
The original patch, revised to meet initial review suggestions
16bit_step2.patch (2.5 KB ) - added by SF/upthorn 10 years ago.
Part 2 -- making the vCUPhe subengine display a 16bit gradient to test step1.
16bit_step3.patch (6.6 KB ) - added by SF/upthorn 10 years ago.
Part 3 -- making the v100he subengine non-conditionally display a 16bit background in freddicove
16bit_step1_r2.patch (7.9 KB ) - added by SF/upthorn 10 years ago.
The original patch, revised to reflect the fact that freddicove uses 555 rgb, whereas most hardware implementations of 16bit are 565.
16bit_step1_r3.patch (7.9 KB ) - added by SF/upthorn 10 years ago.
The original patch, revised to reflect that in full screen updates, it is the width of the rectangle, and not the pitch, that equals the screenwidth.

Download all attachments as: .zip

Change History (20)

by SF/upthorn, 10 years ago

Attachment: 16bit_step1.patch added

The initial patch, unreviewed

comment:1 by sev-, 10 years ago

Owner: set to sev-

comment:2 by sev-, 10 years ago

Good start. I've commented you on IRC. Looking forward to next update

by SF/upthorn, 10 years ago

Attachment: 16bit_step1_r1.patch added

The original patch, revised to meet initial review suggestions

comment:3 by SF/upthorn, 10 years ago

I have revised the initial modification to be compile-time optional (see 16bit_step1_r1.patch), and built off the current svn trunk, instead of a weeks-old revision.

The other suggestion made on IRC concerns a second logical feature, and will be addressed in a second patch, to be submitted when completed (hopefully some time tomorrow.)

comment:4 by lordhoto, 10 years ago

It seems the mouse cursor code is still 8bpp based with your patch. I'm unsure whether you didn't look into reworking that yet or whether it is intentional. IMHO it should be 16bpp though.

by SF/upthorn, 10 years ago

Attachment: 16bit_step2.patch added

Part 2 -- making the vCUPhe subengine display a 16bit gradient to test step1.

comment:5 by SF/upthorn, 10 years ago

I have now modified the scumm vCUPhe subengine to display a 16bpp gradient instead of playing a CUP movie. This is compile-time optional via the ENABLE_16BIT define. (See 16bit_step2.patch)

comment:6 by lordhoto, 10 years ago

The patch is a nice start. The compile-time switch is nice, of course later on it would also be needed to switch on run-time. The compile-time bit could then still be kept, so ports not supporting (or with too low resources) could strip out *all* 16bpp game graphics related code.

I would like to get some more information about how to proceed with the common API update needed for proper 16 bit support.

When do you plan to extend the 16bit API so it could be integrated properly? For example currently there's no way to query the bit depth setup, no way to request a bit depth, some backend code is not updated for 16bpp (mouse cursor still relies on palette for example).

Along with it the common code in graphics/ and gui/ would need updates:

For example when mouse cursor is setup to use 16bpp the GUI code would need to be aware of it and pass its cursor accordingly.

CursorMan would need to be adjusted to support 16bpp cursor in general too.

Another related matter is the thumbnail creating code, it also relies on the game screen being 8bpp and palette based currently. That one can be also be easily changed, when a proper API for querying the setup graphics mode is present.

There are probably other code what might need update I forgot about right now too :-).

That might sound like a quite share of work needed, and it is probably, but IMHO this is an essential part of the 16bit support.

Also I guess your mentor talked about that with you already. Still it would be nice to see some more information on those general API matters.

comment:7 by lordhoto, 10 years ago

Just to make things straight, that's my personal interest. I don't know how you planned your work with your mentor(s), so in case of doubt just ignore it and follow the guidelines from your mentor(s).

by SF/upthorn, 10 years ago

Attachment: 16bit_step3.patch added

Part 3 -- making the v100he subengine non-conditionally display a 16bit background in freddicove

by SF/upthorn, 10 years ago

Attachment: 16bit_step1_r2.patch added

The original patch, revised to reflect the fact that freddicove uses 555 rgb, whereas most hardware implementations of 16bit are 565.

comment:8 by SF/upthorn, 10 years ago

I have now modified the scumm v100he subengine to attempt display of a 16-bit background image from the freddicove demo if the game is listed as having 16bit color in the detection table.
WARNING: It makes no checks to assure that the game is actually freddicove
NOTE: This was built and tested using the freddicove demo wherein FFCoveDemo.HE0 has an MD5 of 45082a5c9f42ba14dacfe1fdeeba819d
This demo is available at http://quick.mixnmojo.com/demos/ffcovedemo.zip

comment:9 by SF/upthorn, 10 years ago

Additionally I have modified the SDL backend very slightly so that the 16-bit game SDL surface uses 555 rgb and not 565, regardless of hardware settings. (see 16bit_step1_r2.patch)

Furthermore I should have noted in the prior comment that the code referred to therein is contained in 16bit_step3.patch, in case that may be non-obvious.

comment:10 by SF/upthorn, 10 years ago

Another minor modification of the SDL backend.
In copyRectToScreen, I had previously failed to properly update the condition which tests for a full screen copy. This is now rectified (see 16bit_step1_r3.patch).

by SF/upthorn, 10 years ago

Attachment: 16bit_step1_r3.patch added

The original patch, revised to reflect that in full screen updates, it is the width of the rectangle, and not the pitch, that equals the screenwidth.

comment:11 by sev-, 10 years ago

Upthorn, as I understand, this was committed now?

comment:12 by SF/upthorn, 10 years ago

Yes, a branch has been created from this patch artifact. I guess that probably means I should close it...

comment:13 by SF/upthorn, 10 years ago

Status: newclosed

comment:14 by digitall, 10 months ago

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