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

Full-screen fill rectangle optimizations #123

Merged
merged 2 commits into from
May 29, 2024

Conversation

Thar0
Copy link
Contributor

@Thar0 Thar0 commented May 28, 2024

Greatly improves per-frame full-screen fill rectangle usage.

Vanilla OoT does the following on each frame:

  • If there is no skybox and no solid background color
    • Fill the screen black
  • If there is a solid background color
    • Fill the screen black
    • Fill the screen with the solid background color
  • If there is a textured skybox
    • Fill the screen black
    • Draw the skybox
    • Fill the screen with a fog filter for the skybox, sun and moon using an XLU render mode

This has been improved to perform less full-screen fill rectangles per frame

  • If there is no skybox and no solid background color
    • Fill the screen black
  • If there is a solid background color
    • Fill the screen with the solid background color
  • If there is a textured skybox
    • Draw the skybox (this acts as the framebuffer clear, with one caveat mentioned below) and use the G_RM_FOG_PRIM_A to do the fog filtering for the skybox, sun and moon all in one pass, massively reducing the consumed memory bandwidth

Also gates the Environment_FillScreen and Environment_DrawLensFlare screen fills with an alpha >= 8 check. The RDP blender uses the 5 most significant bits of alpha for blending, so if alpha were less than 8 it would just end up writing the same color back to the framebuffer. This makes absolutely sure this cannot occur.

There is a caveat that skyboxes with an open bottom will cause a "Hall of Mirrors" effect if the open bottom is ever on screen, since those pixels will end up retaining whatever was last in the framebuffer. It is assumed that there is no case in normal gameplay where this should happen, but maybe it's worth adding a toggle to give these skyboxes solid color bottom faces to avoid this.

I've also made some other minor changes

  • Added a message at the final link step in the makefile. Without this it wasn't clear if the build was still working properly since final link takes a long time.
  • Added editor.rulers to settings.json, this draws a vertical line at the 120 character limit in the editor to remind you where the formatter is going to cut things off at. I personally like it but maybe it's not for everyone, try it out.

Copy link
Collaborator

@Yanis002 Yanis002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to work properly, when you look at the skybox bottom this happens: https://imgur.com/a/YWUOoVJ, I assume this is the new expected behavior? also the linking message looks a bit weird Linking: -> build/hackeroot-mq/hackeroot-mq.elf, I think we might want a print version without the arrow somehow but ti's just a small detail (or alternatively you could remove the : to make it look like Linking -> build/hackeroot-mq/hackeroot-mq.elf)

otherwise looks good to me, really really nice to see these kinds of improvements, that's something I'd never been able to do on my own so thanks for this

@Thar0
Copy link
Contributor Author

Thar0 commented May 28, 2024

when you look at the skybox bottom this happens ... I assume this is the new expected behavior?

Yeah I mentioned this happens in the PR description. This is the only intended deviation from vanilla visuals. My assumption is that in regular gameplay this should never happen, but alternatively we could add a bottom face to skyboxes that's just a solid color to restore vanilla behavior. This would have a performance cost though, so I would suggest if we do this it should have a config option associated to it.

linking message looks a bit weird

Some other existing messages have the same kind of issue so I figure this could be left for a separate cleanup that focuses more on this. It was just bugging me that there was no message at all while building.

Copy link
Collaborator

@sauraen sauraen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would strongly recommend adding a bottom face to the skyboxes before merging this. It can be black or whatever. The "nothing cleared the framebuffer" effect looks very bad IMO and could cause issues for people who are photosensitive as it alternates between two framebuffers. It is not rare to get the camera outside or partially outside the map even in vanilla, and definitely in romhacks.

One other thing: using the skybox to clear will only work if it is using a rendermode with coverage mode set to FULL (also known as ZAP), and it is set up to ignore incoming coverage (no IM_RD and some other bits). I'm pretty sure this is not how the skybox is set up in vanilla and it looks like it hasn't been changed here.

@Thar0
Copy link
Contributor Author

Thar0 commented May 29, 2024

Added the bottom face for skyboxes.

Skyboxes are drawn with the CLAMP mode with IM_RD disabled. Since the skybox is guaranteed to cover the entire screen, this is already sufficient to guarantee that coverage is reset for the frame.

Copy link
Collaborator

@sauraen sauraen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be a little more complicated than that, but if this has been tested on console or Ares and there aren't artifacts on the edges of sky tris, then it should be fine.

@sauraen sauraen merged commit 7a3926c into HackerN64:develop/2.1.0 May 29, 2024
2 checks passed
@Thar0
Copy link
Contributor Author

Thar0 commented May 29, 2024

Ah yeah I missed a key part of the explanation, here it is for the curious:

When force blending (which is the case here) the clamp mode sums old and new coverage before clamping. When IM_RD is disabled old cvg is assumed to be full (7) so the calculation is clamp(7 + c) which of course clamps to 7 for all c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants