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

Texture writing overhaul #194

Merged
merged 52 commits into from
Apr 13, 2023
Merged

Texture writing overhaul #194

merged 52 commits into from
Apr 13, 2023

Conversation

sauraen
Copy link
Contributor

@sauraen sauraen commented Dec 3, 2022

YouTube video "fast64 museum tour!", created as marketing material for this PR

PR which overhauls F3D texture writing. These changes were originally started for the following reasons:

  • Effectively move whether textures are CI and the CI format from being a per-texture property, to a property shared by both textures, as this is how it is implemented on the RDP. The texture formats are still set per-texture, but a GUI warning is shown and an error is thrown at export if one texture is CI while the other is not, or if the CI formats (RGBA16 or IA16) are different. The othermode bits to set CI and CI format are moved from being written separately with each texture to being written with the other othermode bits, saving 1-2 instructions per material DL.
  • Allow for two CI4 textures with different palettes to be used together as multitexture (this works on N64, but in fast64 before this PR, it would try to combine them into one palette and fail if there were more than 16 different colors).

This turned into a full overhaul of the texture writing functions. This includes improved support for:

  • Texture references
  • Flipbook textures
  • Large textures
  • All combinations of the above

Functionality is now separated into the following steps:

  • Fetching basic texture information (for the othermode bits as well as further uses below), and validating some texture properties and conditions.
  • Determining how to arrange CI palettes in the upper half of TMEM, including handling of ALL cases of mixed formats (e.g. CI4 + CI8), texture references, export as PNG mode, and palette merging
  • Determining how to arrange texel data in the lower half (or all) of TMEM (if two textures are the same)
  • Generating or getting texture and palette definitions
  • Writing DL entries, including separate handling of the "load" and "set tile" steps
  • Writing actual texture data

This overhaul was necessary because with the existing code organization, the information needed to make the decisions with CI textures (e.g. whether the union of the colors used in two CI4 images was greater than 16 colors) was only available deep within the functions writing the textures. The new code is also often simpler and more modular.

This PR is not expected to conflict with the material bleeding PR, as I still use the standard GBI classes.

@sauraen sauraen added bug Something isn't working enhancement New feature or request codebase Code maintenance/cleanup f3d Has to do with the "f3d" code common to all games waiting for author Waiting for the author to answer questions, do changes, ... labels Dec 3, 2022
@Zelllll
Copy link
Contributor

Zelllll commented Dec 3, 2022

I actually have a question about this. Do you know if having a single CI texture as both texel0 and texel1 works still?

@Dragorn421 Dragorn421 marked this pull request as draft December 12, 2022 09:54
fast64_internal/f3d/f3d_parser.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_parser.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_parser.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_parser.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_parser.py Outdated Show resolved Hide resolved
Comment on lines 160 to 163
allImages = []
for flipbookTexture in flipbookProp.textures:
if flipbookTexture.image not in allImages:
allImages.append(flipbookTexture.image)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
allImages = []
for flipbookTexture in flipbookProp.textures:
if flipbookTexture.image not in allImages:
allImages.append(flipbookTexture.image)
allImages = list(set(flipbookTexture.image for flipbookTexture in flipbookProp.textures))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this does not work--the set will scramble the order of the items. For multiple materials using the same texture, it matters what order the textures are in the use list. Python is unfortunately missing a native list function to remove duplicates keeping the first instance of each.

existingFPalette = None
fImages = []

palName = model.name + "_" + flipbookProp.textures[0].image.name
Copy link
Contributor

Choose a reason for hiding this comment

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

was removing the _pal suffix intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Names have since been completely overhauled (they're basically what they used to be, including the _pal suffix).

fast64_internal/sm64/sm64_f3d_writer.py Outdated Show resolved Hide resolved
fast64_internal/sm64/sm64_f3d_writer.py Outdated Show resolved Hide resolved
@sauraen sauraen added review code please Ask that some other Fast64 dev reviews the code test please Ask that some other Fast64 dev tests the feature/... and removed waiting for author Waiting for the author to answer questions, do changes, ... labels Feb 14, 2023
@sauraen sauraen marked this pull request as ready for review February 14, 2023 02:08
@sauraen
Copy link
Contributor Author

sauraen commented Feb 14, 2023

This PR works with all kinds of texture formats as tested with the test scene (see the video). However, it needs to be tested with SM64, especially the TexRect feature. If someone who has a SM64 mod could export one of their scenes with this branch and confirm it doesn't break, that would be great.

@sauraen
Copy link
Contributor Author

sauraen commented Feb 17, 2023

bozies exported the museum test scene in SM64 (in an emulator), and it worked fine. If someone could test TexRect that would be great, but otherwise this is reasonably well tested.

@sauraen
Copy link
Contributor Author

sauraen commented Mar 19, 2023

The mat bleed changes are now merged into this, and it is now up to date with top-of-tree. Needs review.

Note that OoT scene exporting currently corrupts scene_table.h. This is an unrelated bug not caused by this PR (it's in main).

@sauraen sauraen requested review from kurethedead, thecozies and Dragorn421 and removed request for Dragorn421, kurethedead and thecozies March 19, 2023 18:46
Copy link
Contributor

@thecozies thecozies left a comment

Choose a reason for hiding this comment

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

Mostly just requests for some small cleanup but nothing blocking in of itself. There seems to be an issue with mat bled textures, both with scrolling and large textures but i havent figured out exactly whats going on. I'll post a separate comment with info.

I can see some potential issues with some of the messaging to users. I think the large texture creator may be a little overwhelming in its language to some users. Not a blocker, but something we should respond to as we get user feedback on the feature.

fast64_internal/f3d/f3d_bleed.py Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Outdated Show resolved Hide resolved
fast64_internal/f3d/f3d_texture_writer.py Show resolved Hide resolved
fast64_internal/f3d/f3d_writer.py Show resolved Hide resolved
bugfix for large texture mode during mat bleed
@sauraen sauraen merged commit 27a7ea6 into Fast-64:main Apr 13, 2023
@@ -12,26 +13,27 @@ class TextureFlipbook:
name: str
exportMode: str
textureNames: list[str]
images: list[tuple[bpy.types.Image, FImage]]
Copy link
Contributor

Choose a reason for hiding this comment

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

This change was made without updating a bunch of TextureFlipbook instanciations such as
TextureFlipbook(arrayName, "Array", textureList)
which breaks a bunch of code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working codebase Code maintenance/cleanup enhancement New feature or request f3d Has to do with the "f3d" code common to all games review code please Ask that some other Fast64 dev reviews the code test please Ask that some other Fast64 dev tests the feature/...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants