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
Don't hardlink texture atlas metadata into the executable #12656
Conversation
Hm, there's still something off, with the PPGe one at least - a (tm) symbol becomes a triangle. |
ddb3e2d
to
9ce1236
Compare
Solved the problem, output is correct now. In the future I might devise a more compact metadata format and even tack it onto the end of the .zim files, but want to get this in now. |
SLN fix It works, but with the wrong images and the wrong characters! Fix another bug in atlastool's binary output Get Android building again. Oops, didn't mean to disable this permanently. Error checking Minor cleanup Gotta tweak my git ignores... Regenerate metadata
(Due to the squash and some reordering, the previous commit already includes fixed metadata)
9ce1236
to
4c392c6
Compare
@@ -144,8 +144,13 @@ void PSPDialog::DoState(PointerWrap &p) | |||
p.Do(isFading); | |||
p.Do(fadeIn); | |||
p.Do(fadeValue); | |||
|
|||
// I don't think we should save these two... Let's just ignore them for now for compat. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, as long as we restore them properly per messageDialog.common.buttonSwap
/ param.GetPspParam()->common.buttonSwap
? I guess it should correct on next Update.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it was always wrong to store IDs as part of a save state here that depend on what values the texture atlas compiler assigned.
I think it should indeed be enough to wait for the next update.
AtlasWordWrapper wrapper(*atlas->fonts[font], fontscalex, toMeasure.c_str(), bounds.w); | ||
const AtlasFont *font = atlas->getFont(font_id); | ||
if (!font) | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, should still set w/h to 0?
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, will fix.
With this change, atlas metadata (the positions and names of the sub-images) is now in a separate file, so if you rebuild the UI atlas, PPSSPP will no longer need to be rebuilt in order to see the results, even if you add stuff.
In addition, accidentally using the wrong version of the assets will no longer result in visual garbage, in future builds (but maybe some missing visuals). And we get rid of the horrible generated cpp file.
This will improve the workflow when adding images to PPSSPP's UI. So hopefully it'll be easier to find the energy to do some UI work... And in the future, maybe even dynamic atlas.
I don't think the tiny, tiny perf hit is worth worrying about - maybe we should lookup the names by hash but meh...
Don't merge yet, some more cleanup left, just wanna run CI. Might also compress the metadata files some - they're very low entropy as-is.