#8611 closed patch (outdated)
Support for a 32 bit _hwscreen in the SDL backend
| Reported by: | DrMcCoy | Owned by: | DrMcCoy |
|---|---|---|---|
| Priority: | low | Component: | Ports |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
This is a patch to add a 32 bit _hwscreen (if the display can handle this color depth). My motivation was that some images in Ween: The Prophecy use smooth gradients and the 16 bit surface destroys them (bug #1662625). (My initial plan from 2 days ago was using 24 bit, but then I saw that 32 bit depth is far more easier to integrate into the current code.)
This *shouldn't* break anything (it's optional, the default is still 16 bit and the only changes are in the SDL backend and the scalers; everything works for me at least), but save's save, so I'd like to know beforehand (especially from the porters) whether it is indeed correct, there would be any problems (on other systems using the SDL backend, for example), etc..
Two disadvantages though: It introduces some code-dup (in graphics/scaler/intern.h and graphics/scaler/aspect.cpp) and two more tmpscreens in the SDL backend...
It also still switches back to 16 bit when using the HQ-Scalers (but from what I've seen, the orignal scaler converts the image to 16 bit before scaling, too).
Ticket imported from: #1664514. Ticket imported from: patches/716.
Attachments (2)
Change History (8)
by , 18 years ago
| Attachment: | 32bit.patch added |
|---|
comment:1 by , 18 years ago
Some comments:
* In sdl/graphics.cpp, you should add a comment explaining why you set _Rmask = 0xF800; etc. explicitly in the 32 bit case, instead of using _hwscreen->format->Rmask etc.
* A comment stating why and when which BPP is chosen would be very helpful
* You should document why the HQx scalers are treated differently (likely because of the YUV/LUT lookup tables, but that shouldn't be left to guesses :)
* Two more tmp screens? OUCH, that's not nice at all. Means a lot more memory, and an even slower rendering pipeline... or not? Hard to say, as it's not clear why those new surfaces are needed etc.. The rendering pipeline is already now badly documented, but with your changes, it gets worse :-/. Please try to explain in a tracker item comment why these are needed. Then we can either try to optimize them away, or at least add a comment explaining them.
* With your change, the names "interpolate32_1_1_1_1" vs. "interpolate16_2" become misleading. The former was once named so because it can correctly take *two* 16pixels in each of the four values, and interpolate them -- i.e. it can process *8* 16 bit input pixels to produce two output pixels. This trick of course doesn't work once one allows 32bit pixel data (unless one runs on a 64 bit machine).
The same holds for the other interpolate32_FOO functions. The patch currently causes these two have different semantics when in 32 bit mode, which is IMO a bad idea, and will easily lead to nasty bugs. We should resolve this.
One way to do so: We could try to get rid of the interpolate32_FOO funcs completely. About the only code where it would be sad to loose them is the aspect ratio code (the kFastAndNiceAspectMode portion of it). My proposal: Let's get rid of kSlowAndPerfectAspectMode and kVeryFastAndUglyAspectMode, clean up the remaining code, then directly insert the code of the current interpolate32_FOO templates there, where appropriate
comment:2 by , 18 years ago
Attached a second try, which addresses the missing comments and misleading interpolation function names you mentioned.
About the additional temp screens: The overlay is still always a 16 bit surface (changing it to a 32 bit one would mean changing OverlayColor and probably more), so the two additional temp screens are needed to convert the overlay image between 16 and 32 bit when the overlay is visible. Otherwise, the scalers would have to cope with different "depthed" source and destination surfaces. Again, that's maybe not the best way to handle this, just the only one I could think of without affecting other code than the SDL backend and the scalers. File Added: 32bit_try02.patch
comment:3 by , 18 years ago
Note that the gradient in the launcher dialog also suffers in 15/16 bit mode, so it would benefit from a 32bit overlay (or alternatively, from dithering). (This is not meant to imply that 32 bit overlays should be implemented, I am simply recording this observation here for completness).
comment:4 by , 17 years ago
So to implement this on the long run, we need to get rid of OverlayColor, and change the overlay to be a Graphics::Surface with a generic "Graphics::PixelFormat" (a struct which has yet to be introduced, modelled after SDL_Pixelformat), so that we can make the GUI graphics code generic (i.e. so that it supports 32 bit overlays).
comment:5 by , 15 years ago
| Resolution: | → outdated |
|---|---|
| Status: | new → closed |
comment:6 by , 6 years ago
| Component: | → Ports |
|---|
The patch