Background and screencapture rendering fixes #347
Conversation
|
Do all instances of bgsprite need to be replaced with rendersprite then? |
Not all of them. |
PringlesGang
force-pushed
the
bg-fixes
branch
from
a44f87a to
8e96d76
Compare
PringlesGang
force-pushed
the
bg-fixes
branch
from
8e96d76 to
c104b24
Compare
Enorovan
left a comment
Enorovan
left a comment
There was a problem hiding this comment.
No visual regressions on Triggers, BG Movements, SetLinks, Sprites, Polygonal Effects, Sprites, Capture Events ✅
| for (int i = static_cast<int>(Profile::ScreenCaptureCount) - 1; i >= 0; i--) { | ||
| const int capOffset = i * Profile::Vm::ScrWorkCaptureEffectInfoStructSize; | ||
| if (ScrWork[SW_EFF_CAP_BUF + capOffset] == GetScriptBufferId(bgId + 1)) { | ||
| RenderSprite = Screencaptures[i].BgSprite; |
There was a problem hiding this comment.
instead of having two sprites, what if we just have a single sprite and update the spritesheet when necessary to the screencap sheet?
There was a problem hiding this comment.
That would necessitate storing the SpriteSheet separately in each Background, as currently BgSprite is the only thing holding on to that. That ok?
There was a problem hiding this comment.
Yeah that should be fine. You're already duplicating spritesheet by duplicating sprite, so there should just be less to dupe now.
There was a problem hiding this comment.
Idk man, I've just tried to dedupe the stuff, but the Sprite actually adds crucial information about the texture beyond what is already stored in the SpriteSheet
Mainly the BaseScale is important here, as for the captures and renderbuffers this represents the ratio between the window size and the design size, which is information that is not stored in the SpriteSheet but is crucial for how the texture is supposed to be rendered
Technically this could be set every time when the RenderSprite would be set, but this would also necessitate resetting it to either glm::vec2(1.0f) every time in case no capture is selected (and this is assuming there aren't ever any backgrounds that have a different BaseScale); or to a separately stored base scale, still duping it
There's an argument to be made for the Bounds, which is needlessly duped, but I don't think that fully justifies the above drop in code quality :/
tldr, I think the two sprites truly carry different semantic meaning: BgSprite is the full sprite held by the class, denoting everything that the class could render on that sheet, whereas RenderSprite denotes what part of that (or another) sheet should be rendered that frame
There was a problem hiding this comment.
Sure, you can just leave it as bgsprite then
Rewrites core background and screencapture logic to more closely reflect its behaviour in the binary
CHLCC never renders its captures separately; always through a background object.
CCLCC does render its captures separately, along with through background objects.
This used to be handled by
Vm::Interface::UpdateBackground2D, but that implementation was flawed, as when a background is rendered separately,SW_CAPvariables are used, whereas when it's rendered through a background object, the correspondingSW_BGvariables are used instead.The distinction between background buffer and background id was also sometimes mishandled.
Between the two engine revisions, the
SW_BG1DISPMODEvariable also handles the background transformations differently. The class now properly changes behaviour based on the revision of the engine it is meant to emulate.