#4880 closed defect (fixed)
IPHONE: No grabPalette() implementation
| Reported by: | SF/mthreepwood | Owned by: | lordhoto |
|---|---|---|---|
| Priority: | normal | Component: | Port: iOS |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
The iPhone backend doesn't implement the grabPalette() function, which could possibly lead to problems in the engines that use that OSystem function: Tucker, Kyra, M4, Groovie, and SCI
Ticket imported from: #2999153. Ticket imported from: bugs/4880.
Attachments (1)
Change History (15)
by , 14 years ago
| Attachment: | iphone-grabPalette.patch added |
|---|
comment:1 by , 14 years ago
I just attached a patch which tries to implement grabPalette. Right after that, I realized that I actually recently installed the iPhone SDK, so I can test it -- did just that, and found that I made a mistake.
Anyway, since it compiles, I feel confident enough to just commit it ... ;). But I am not closing this item yet, as vinterstum should review (and ideally test?) this change.
comment:2 by , 14 years ago
Doesn't that lose precision on the RGB values? The iPhone backend converts from 24-bit to 16-bit on setPalette() and now 16-bit to 24-bit on grabPalette().
comment:3 by , 14 years ago
It is true that this "looses" bits compared to what is passed in via setPalette. But nowhere in the OSystem specs does it say that grabPalette will return the exact same data that was set via setPalette, It only promises to return the "currently active palette", and that it does: If you call grabPalette and then setPalette with the data returned by grabPalette, the palette stays unchanged (we don't say this explicitly, but it is implicit from the wording of the grabPalette specs, I'd say).
Note that the dc, ds, n64, ps2 and wii backends do the exact same thing (and also the psp backend, unless used in 32bit mode).
Only the SDL backend, and backends derived from it (WinCE, Symbian, SamsungTV, ..., and indirectly GP2x) actually always prefer the data passed in.
comment:4 by , 14 years ago
It seems wrong that this is 'assumed' to return different than what is passed in. I also don't believe this is implicitly stated in the documentation, there's nothing that says either way; it just says "currently active palette" which makes sense to be assumed to be the same palette that was set from the engines.
I could come up with a patch that would switch them all to storing the palette in 32-bits (of course, I wouldn't have a way of testing this on any of the other ports as of this writing).
comment:5 by , 14 years ago
In the very least, the documentation for grabPalette() could use an update ;)
comment:6 by , 14 years ago
This seems to have been implemented already by fingolfin, right? Can it be closed now?
comment:7 by , 14 years ago
| Owner: | changed from to |
|---|
comment:8 by , 14 years ago
I made a change there, yes, but I can't test it myself, and I don't know if it works at all. So I'd really prefer if somebody would verify whether this actually works now.
comment:9 by , 13 years ago
| Owner: | changed from to |
|---|
comment:11 by , 13 years ago
A good test case would be a game that uses grabPalette(). An example that comes to mind is SCI, whenever a fadein/fadeout or a palette variation effect is made, like, for example, in Freddy Pharkas, when starting a new game. There's an effect which changes the screen colors from sepia to full color, which uses grabPalette().
comment:12 by , 13 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:13 by , 13 years ago
LordHoto has been working on the IPhone backend and has reviewed the relevant code. He is happy that it looks good so closing. If testing reveals any issues, this can be reopened.
comment:14 by , 6 years ago
| Component: | → Port: iOS |
|---|
Attempt to implement grabPalette