Implement Texture Scrolling Interpolation#969
Conversation
Removed the macro for setting tile size interpolation.
Added TextureInterpValues structure and updated GfxDpSetTileInterp function to use it.
| int32_t GetMouseCaptureScancode(); | ||
| void SetFullscreenScancode(int32_t scancode); | ||
| void SetMouseCaptureScancode(int32_t scancode); | ||
| uint64_t mFrameCount; |
There was a problem hiding this comment.
invalid case style for public member mFrameCount
| uint64_t mFrameCount; | |
| uint64_t MFrameCount; |
|
This version seems to be a tad smoother since its based off of the actual FPS rate instead of interpolation rate. However, it requires upping your step value by ~1000 bool gfx_scroll_texture_handler_rdp(F3DGfx** cmd0) {
F3DGfx* cmd = *cmd0;
Interpreter* gfx = mInstance.lock().get();
uint64_t frameCount = Ship::Context::GetInstance()->GetWindow()->mFrameCount;
uint32_t tile = C0(0, 12);
float incX, incY;
int32_t stepX = (int32_t) C1(32, 32);
int32_t stepY = (int32_t) C1( 0, 32);
++(*cmd0);
float uls = *reinterpret_cast<float*>((((u8*)&(*cmd0)->words.w0)) + sizeof(float));
float ult = *reinterpret_cast<float*>(&(*cmd0)->words.w0);
float lrs = *reinterpret_cast<float*>((((u8*)&(*cmd0)->words.w1)) + sizeof(float));
float lrt = *reinterpret_cast<float*>(&(*cmd0)->words.w1);
incX = (float)stepX / (float)(ImGui::GetIO().Framerate);
incY = (float)stepY / (float)(ImGui::GetIO().Framerate);
//printf("incX %f incY %f fps %d fpsIndex %d\n", incX, incY, gfx->mTargetFps, gfx->mFpsIndex);
uls += incX;
ult += incY;
lrs += incX;
lrt += incY;
uls = std::fmod(uls, 2048);
ult = std::fmod(ult, 2048);
lrs = std::fmod(lrs, 2048);
lrt = std::fmod(lrt, 2048);
// Apply values to texture
gfx->mRdp->texture_tile[tile].uls = uls;
gfx->mRdp->texture_tile[tile].ult = ult;
gfx->mRdp->texture_tile[tile].lrs = lrs;
gfx->mRdp->texture_tile[tile].lrt = lrt;
// This isn't strictly necessary, but a port can
// overwrite the tile size command making this necessary
gfx->mRdp->textures_changed[0] = true;
gfx->mRdp->textures_changed[1] = true;
//printf("Scroll: uls %f ult %f lrs %f lrt %f incX %f incY %f fps %d interpCount %d\n", uls, ult, lrs, lrt, incX, incY, gfx->mTargetFps, gfx->mInterpolationCount);
(*cmd0)->words.w0 = ((uintptr_t) *(uint32_t*)&uls << 32 | *(uint32_t*)&ult);
(*cmd0)->words.w1 = ((uintptr_t)*(uint32_t*)&lrs << 32 | *(uint32_t*)&lrt);
return false;
} |
briaguya0
left a comment
There was a problem hiding this comment.
my main concerns here are the breaking changes. i know the PR description includes some good examples of how to use this, but it'd be good to have more of a "migration guide" (if you are currently doing X, when this PR lands you should do Y)
the most scary breaking change is the opcode reuse, i really want to hear from people using G_SETTILESIZE_INTERP because with this PR that'll be interpreted as G_SCROLL_TEXTURE which can wreak havoc
| uint32_t mInterpolationIndex; | ||
| uint32_t mInterpolationCount; |
There was a problem hiding this comment.
we need to verify no ports are relying on being able to pass in negative values here
There was a problem hiding this comment.
The interpolation index is generally a value from 0-16
| constexpr int8_t OTR_G_LOAD_SHADER = OPCODE(0x43); | ||
| constexpr int8_t RDP_G_SETTILESIZE_INTERP = OPCODE(0x44); | ||
| constexpr int8_t RDP_G_SETTARGETINTERPINDEX = OPCODE(0x45); | ||
| constexpr int8_t RDP_G_SCROLL_TEXTURE = OPCODE(0x44); |
There was a problem hiding this comment.
this is very much a breaking change. i'd want to either see a migration guide for people using the previous opcode defs or for this to be handled in a non-breaking way
| #define G_LOAD_SHADER 0x43 | ||
| #define G_SETTILESIZE_INTERP 0x44 | ||
| #define G_SETTARGETINTERPINDEX 0x45 | ||
| #define G_SCROLL_TEXTURE 0x44 |
There was a problem hiding this comment.
| #define gDPSetInterpolation(pkt, index) \ | ||
| _DW({ \ | ||
| Gfx* _g = (Gfx*)(pkt); \ | ||
| \ | ||
| _g->words.w0 = G_SETTARGETINTERPINDEX << 24; \ | ||
| _g->words.w1 = index; \ | ||
| #define gDPScrollTexture(pkt, t, uls, ult, lrs, lrt, stepX, stepY) \ | ||
| _DW({ \ | ||
| Gfx* _g = (Gfx*)(pkt); \ | ||
| if (pkt) \ | ||
| ; \ | ||
| _g->words.w0 = (_SHIFTL(G_SCROLL_TEXTURE, 24, 8) | _SHIFTL(tile, 0, 12)); \ | ||
| _g->words.w1 = (_SHIFTL(stepX, 32, 32) | _SHIFTL(stepY, 0, 32)); \ | ||
| _g++; \ | ||
| _g->words.w0 = (_SHIFTL(uls, 32, 32) | _SHIFTL(ult, 0, 32)); \ | ||
| _g->words.w1 = (_SHIFTL(lrs, 32, 32) | _SHIFTL(lrt, 0, 32)); \ | ||
| }) | ||
|
|
||
| #define __gDPSetTileSizeInterp(pkt, t, uls, ult, lrs, lrt) \ | ||
| gDPLoadTileGeneric(pkt, G_SETTILESIZE_INTERP, t, uls, ult, lrs, lrt) | ||
| #define gsDPScrollTexture(t, uls, ult, lrs, lrt, stepX, stepY) \ | ||
| { \ | ||
| (_SHIFTL(G_SCROLL_TEXTURE, 24, 8) | _SHIFTL(tile, 0, 12)), \ | ||
| (_SHIFTL(stepX, 32, 32) | _SHIFTL(stepY, 0, 32)), \ | ||
| }, \ | ||
| { \ | ||
| (_SHIFTL(uls, 32, 32) | _SHIFTL(ult, 0, 32)), (_SHIFTL(lrs, 32, 32) | _SHIFTL(lrt, 0, 32)), \ | ||
| } | ||
|
|
There was a problem hiding this comment.
- breaking changes (same comment as https://github.com/Kenix3/libultraship/pull/969/changes#r2678862060)
- there are some hardcoded magic numbers in here which makes me think this is removing control/flexibility from ports.
| int32_t GetMouseCaptureScancode(); | ||
| void SetFullscreenScancode(int32_t scancode); | ||
| void SetMouseCaptureScancode(int32_t scancode); | ||
| uint64_t mFrameCount; |
There was a problem hiding this comment.
tidy is right to be mad about this. it shouldn't be public. make it private and add a public getter and from what i can tell you should be able to use a protected setter
| } | ||
|
|
||
| void Fast3dWindow::HandleEvents() { | ||
| mFrameCount += 1; |
There was a problem hiding this comment.
why is this in HandleEvents?
There was a problem hiding this comment.
tons of breaking changes in here, same comment as https://github.com/Kenix3/libultraship/pull/969/changes#r2678862060

First of all... Actually, hold on a second.
Merry Christmas! I hope everyone is hanging out with their family, and safe travels to everyone travelling during this time. Stay safe, stay warm (It's about to be -50 celcius where I'm from that's -58 in freedom units). And I hope you're playing a plethora of Christmas carols to annoy every family member who has been sick of hearing them play in the grocery store since September. Okay, with that out of the way.
First of all, I've had very little sleep. I'm tired, I'm emotional, I just want to go home. And as such, I'm going to need a little bit of grace on my next point (...and all the other ones as well).
Second of all, 2Ship can go stick their heads in a snowbank*.
Third of all, this is still wip and it may not quite work yet please provide as helpful suggestions as possible as we work together to find a solution that works for everyone. However, please refrain from statements such as I don't like this because 2ship will have to do more work or 2ship already implemented tex scrolling or I don't like this PR because I'm only getting coal and socks for Christmas because these are not very helpful statements, and why should I care anyway? You probably deserved the coal. Socks on the other hand, oooh, wouldn't wish that on my worst enemy, but still, sucks to suck!
I intend to do the following:
*Please do not actually go stick your head in a snowbank. (I've been informed not to say this, but f it) No matter how much I wish you would
Todo
Possible Improvements
Port-side Implementation Instructions
Add this line beside mInterpolationIndex
Normal DL Example
Replace gDPSetTileSize with:
Static DL Example
Run this once at the start of your level. Be sure to that the life time of writableDList outlasts the level and draw phase otherwise it will crash if it becomes null
Modify Scroll After Created Static DL Example
Possible Delta Time Solution
This solution didn't work because it locked the scroll speed into the framerate. Whereas this was mostly solved, it was still a tiny bit effect and didn't have a noticeable improvement over frameCount. It's also likely that the deltaTime function used by SpaghettiKart needs to be ported as it still uses the old macro for OS Time
If this was to be re-looked at, suggest waiting for Kenix tick/draw component PR and then having a deltaTime calculation baked into lus
Troubleshooting
Note that SETTILESIZE command will over-write the values and likely reset the scrolling texture.
It's recommended to replace the tile size command with the texture scroll command.