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

Rendermode internal simplification #271

Merged
merged 13 commits into from
Feb 15, 2024
Merged

Rendermode internal simplification #271

merged 13 commits into from
Feb 15, 2024

Conversation

sauraen
Copy link
Contributor

@sauraen sauraen commented Jan 1, 2024

Like some of my other PRs, this started as a simple fix--make the fog controls show up under Sources if fog color or fog alpha are being used in the rendermode, even if fog is not enabled in the geometry mode. This turned into three main changes:

  • fast64 now sets all the individual options for the advanced rendermode (while they're hidden) based on which rendermode presets are selected. This conversion happens at .blend file load and when changing the presets. This does not affect export (the strings for the presets are still written if the presets are used).
  • Code dependent on the rendermode now is significantly simplified, because it only has to check the individual advanced rendermode settings, rather than have one codepath for that and another to extract the relevant bits from the selected presets.
  • A GUI warning is added if the selected rendermode presets are not valid (exactly one must set the rendermode flags). The warnings about RDP silicon bugs are moved from being shown for advanced mode only, to also be shown when the presets are in use. The combination of these warnings will cover most cases of guiding users towards setting up the blender correctly.

@sauraen sauraen requested a review from Yanis002 January 1, 2024 00:38
Copy link
Contributor

@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.

I still need to try this but code looks ok to me, I don't have a lot to say about it

fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/oot/oot_upgrade.py Show resolved Hide resolved
@Dragorn421
Copy link
Contributor

Dragorn421 commented Jan 6, 2024

Good changes!

While checking the material presets to see if they're consistent with the warnings you added, I noticed

oot_shaded_multitexture_lerp_transparent_vertex_alpha
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_OPA_SURF'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

oot_shaded_texture_transparent_vertex_alpha
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_OPA_SURF'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

oot_shaded_vcol_multitexture_lerp_transparent_vertex_alpha
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_OPA_SURF'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

oot_shaded_vcol_texture_transparent_vertex_alpha
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_OPA_SURF'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

And unrelated but this is weird as well? noop makes clr_in 0 for the next cycle

oot_water_lerp_specular_fresnel
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_NOOP'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

oot_water_mult_specular_fresnel
f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_NOOP'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

EDIT: actually these last two have rendermode_advanced_enabled = True so I guess the preset doesn't matter, but it still looks weird 😅

fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_material.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_material.py Show resolved Hide resolved
@Dragorn421
Copy link
Contributor

for the merge conflicts, same suggestion as #280 (comment)

@sauraen
Copy link
Contributor Author

sauraen commented Jan 9, 2024

I apologize for what's going to be a delay--I see you have a couple more comments which didn't show up in the code viewer page for some reason, which I haven't addressed. I'm going to be out of town for a bit soon so it'll be a while before I can get back to this.

@sauraen
Copy link
Contributor Author

sauraen commented Jan 31, 2024

@Dragorn421

f3d_mat.rdp_settings.rendermode_preset_cycle_1 = 'G_RM_OPA_SURF'
f3d_mat.rdp_settings.rendermode_preset_cycle_2 = 'G_RM_AA_ZB_XLU_SURF2'

From gbi.h:

#define RM_OPA_SURF(clk)                                        \
    CVG_DST_CLAMP | FORCE_BL | ZMODE_OPA |                      \
    GBL_c##clk(G_BL_CLR_IN, G_BL_0, G_BL_CLR_IN, G_BL_1)

So this is doing G_RM_PASS for the first cycle, CVG_DST_CLAMP and ZMODE_OPA are both 0 so they don't affect anything when being OR'd in, the only thing it actually sets is FORCE_BL which is also set by all XLU_SURF rendermodes. So practically it is fine, but theoretically it's wrong because it's two rendermodes which set the flags, I've changed all these to G_RM_PASS instead.

noop makes clr_in 0 for the next cycle

No? It sets the four entries to "enum item 0" for each, which is G_BL_CLR_IN, G_BL_A_IN, G_BL_CLR_IN, G_BL_1MA. This is not the same as G_BL_0 which has a value of 3. RM_NOOP is functionally equivalent to RM_PASS though not actually the same configuration.

Copy link
Contributor

@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.

I reviewed again and tested, import works but fog preview has issues I think

Here's what Fast64 shows me with the intro setup (first cutscene header of hyrule field)

Here's how it is in-game:

I'm not sure if this was already broken or not but I noticed preview looked different so I wanted to try

Otherwise the code itself is ok to me

@sauraen
Copy link
Contributor Author

sauraen commented Feb 12, 2024

The difference between Fast64 and in game is present in main as well, so that won't be addressed in this PR. However, there was a bug where rendermode bits were not updated from rendermode presets when creating a material from a preset in code, which led to collision meshes not having fog on this branch. This has been fixed, now the fog appearance in Blender of an imported scene looks the same between main and this branch.

@sauraen sauraen requested a review from Yanis002 February 12, 2024 00:11
@Dragorn421 Dragorn421 added f3d Has to do with the "f3d" code common to all games codebase Code maintenance/cleanup labels Feb 12, 2024
Copy link
Contributor

@Dragorn421 Dragorn421 left a comment

Choose a reason for hiding this comment

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

@Yanis42 ?

Copy link
Contributor

@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.

sorry for the delay

@Yanis002 Yanis002 added the merge soon Will be merged in a few days at most if nothing else comes up label Feb 14, 2024
@sauraen sauraen merged commit 3d6f730 into Fast-64:main Feb 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codebase Code maintenance/cleanup f3d Has to do with the "f3d" code common to all games merge soon Will be merged in a few days at most if nothing else comes up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants