Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TONY: Switch to 565 screen format. #438

Merged
merged 1 commit into from Oct 28, 2014

Conversation

Projects
None yet
8 participants
@fuzzie
Copy link
Member

commented Feb 14, 2014

The old (555) screen format is not supported by some backends.

This leaves the savegame thumbnails as 555 (for compatibility).

@PaulGilbert

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2014

Without having looked at the inner details of the pull request yet, am I to understand that savegame thumbnails will still be an issue for these backends? If that's the case, the savegame format has a version byte already, so you could always bump up the version, and have the new version always use 565, with a bit of logic for handling loading older-style 555 in earlier savegames.

@lordhoto

This comment has been minimized.

Copy link
Member

commented Feb 15, 2014

Without having looked at the inner details of the pull request yet, am I to understand that savegame thumbnails will still be an issue for these backends?

No, our GUI will convert any thumbnails to the native overlay format before displaying them. Thus, it's irrelevant what color format they have.

@fuzzie

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2014

Right, and the conversion for the thumbnails seems to work just fine. I played through a bit so I think/hope this is good, although I'm not quite sure how the sepia toning (in createBWPrecalcTable) is meant to work, so if someone could explain that it'd be nice. The original source code always only uses 5 bits per channel there, so I've probably converted it wrong here.

@bluegr

This comment has been minimized.

Copy link
Member

commented Feb 16, 2014

Looks good at a first glance

@dreammaster

This comment has been minimized.

Copy link
Member

commented Feb 23, 2014

I had a chance to play with it a bit, and it all seems fine except for one minor problem: unlike in master, if you get into a conversation, such as with the two inspectors, highlighting a choice and then moving the mouse off it doesn't properly erase the highlight anymore. Presumably there's still a single remaining uncorrect bitshift in the conversation code.

@dreammaster

This comment has been minimized.

Copy link
Member

commented Mar 10, 2014

Any word on this?

@fuzzie

This comment has been minimized.

Copy link
Member Author

commented Mar 10, 2014

I know what's broken but haven't fixed it yet. :-) Later this week I hope. Thanks for looking at it.

@sev-

This comment has been minimized.

Copy link
Member

commented Apr 28, 2014

Any update?

@dreammaster

This comment has been minimized.

Copy link
Member

commented May 21, 2014

@fuzzie - Any chance of this being finished? You said you knew the cause of the issue, so it is hard to fix?

TONY: Switch to 565 screen format.
The old (555) screen format is not supported by some backends.

This leaves the savegame thumbnails as 555 (for compatibility).
@fuzzie

This comment has been minimized.

Copy link
Member Author

commented Jun 17, 2014

The conversation bug seems to have been caused by RMGfxBox::setColor, but I'm not sure how this ever worked properly in the original.

Anyway, I fixed these two issues (sepia and conversations) and it looks fine to me now.


pixel = (r << 11) | (g << 6) | b;

WRITE_LE_UINT16(&buf[i], pixel);

This comment has been minimized.

Copy link
@clone2727

clone2727 Jun 17, 2014

Contributor

I'm not familiar enough with Tony to tell, but this either breaks or fixes Tony on BE. It previously wrote in native endianness back to the buffer.

This comment has been minimized.

Copy link
@fuzzie

fuzzie Jun 17, 2014

Author Member

Since digitall removed this in 43520ce, I guess this breaks Tony on BE. Agh. Thanks.

This comment has been minimized.

Copy link
@digitall

digitall Jun 17, 2014

Member

I think just change line 1961 to "buf[i] = pixel;" and that should fix it as per 43520ce.

@bluegr

This comment has been minimized.

Copy link
Member

commented Jun 24, 2014

So, when the endianess issue has been addressed, this should be OK to merge, right?

@dreammaster

This comment has been minimized.

Copy link
Member

commented Jun 24, 2014

This was the only issue I had with the original pull request, so if this can be resolved, I'm all for merging.

@RichieSams RichieSams force-pushed the scummvm:master branch from 807d18e to 2ec3adf Sep 12, 2014

@digitall digitall force-pushed the scummvm:master branch from 2208eda to 5b5a2bc Oct 26, 2014

@bluegr

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

It's been 4 months since the last update, so I'd just like to ask: could we merge this, and then fix the endianess issue in-tree?

@PaulGilbert

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2014

I'm actually slightly confused. Re-reading the comment history, it seems like fuzzie fixed the color corruption in the notebook on June 17. If all that's remaining is a possible BE problem, then I can easily consider that a separate issue that could be fixed in-tree, and am all for merging this.

@sev-

This comment has been minimized.

Copy link
Member

commented Oct 28, 2014

I agree, so let it not rot. Merging.

sev- added a commit that referenced this pull request Oct 28, 2014

Merge pull request #438 from fuzzie/tony-565
TONY: Switch to 565 screen format.

@sev- sev- merged commit 3884331 into scummvm:master Oct 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.