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

feat: Upstream Vivy material system #600

Open
wants to merge 41 commits into
base: dev
Choose a base branch
from

Conversation

StandingPadAnimations
Copy link
Collaborator

Vivy is a new material system that is based around the concept of templates instead of generation. I created it in my spare time in 2023 and used it for my new year's piece

https://www.standingpad.org/posts/2023/12/new-years/#vivy

Initially, I have said that I didn't want to upstream Vivy in MCprep to avoid users from opening bug reports after underestimating the complexity of using the system. That said, #274 already had plans to revamp the material system for greater user flexibility, and I'm unable to maintain Vivy on my own (given the fast pace of upstream MCprep). Therefore, I've decided to begin the work of upstreaming Vivy into MCprep itself.

I initially tried to merge Vivy into a separate branch, but that turned out to be impossible due to massive changes in mainline MCprep. Thus, I've included the Vivy files here and added the necessary changes to conf.py and load_modules.py. This was made easier by the fact that Vivy does not change anything internal, mostly due to laziness on my part (instead, I just copied and pasted functions I needed to modify for Vivy). As such, everything in Vivy is a separate operator, using the vivy prefix instead of mcprep. I've not included the necessary reference changes for Vivy, since I don't want to start overriding operators yet.

As it stands, Vivy is lacking in the following areas:

  • UI for setting up materials
  • Documentation for using
  • Integration in the rest of MCprep (like mob rigs)

In addition, Vivy also has the following issues due to the way its designed:

  • Use of VIVY and vivy prefixes
  • Lose integration (again, due to laziness on my part)
  • No tests

Seeing as this is more of a long term project however, I think we can get these tackled sometime by MCprep 4.0. Once those are done, we'd also need to discuss how this would coexist with the existing generation based system (should we choose to keep it).

Closes #274

Vivy is a new material system that is based around the concept of
templates instead of generation. I created it in my spare time
in 2023 and used it for my new year's piece

https://www.standingpad.org/posts/2023/12/new-years/#vivy

Initially, I have said that I didn't want to upstream Vivy in MCprep to
avoid users from opening bug reports after underestimating the
complexity of using the system. That said, #274 already had plans to
revamp the material system for greater user flexibility, and I'm unable
to maintain Vivy on my own (given the fast pace of upstream MCprep).
Therefore, I've decided to begin the work of upstreaming Vivy into
MCprep itself.

I initially tried to merge Vivy into a separate branch, but that turned
out to be impossible due to massive changes in mainline MCprep. Thus,
I've included the Vivy files here and added the necessary changes to
`conf.py` and `load_modules.py`. This was made easier by the fact that
Vivy does not change anything internal, mostly due to laziness on my
part (instead, I just copied and pasted functions I needed to modify for
Vivy). As such, everything in Vivy is a separate operator, using the
`vivy` prefix instead of `mcprep`. I've not included the necessary
reference changes for Vivy, since I don't want to start overriding
operators yet.

As it stands, Vivy is lacking in the following areas:
- UI for setting up materials
- Documentation for using
- Integration in the rest of MCprep (like mob rigs)

In addition, Vivy also has the following issues due to the way its
designed:
- Use of `VIVY` and `vivy` prefixes
- Lose integration (again, due to laziness on my part)
- No tests

Seeing as this is more of a long term project however, I think we can
get these tackled sometime by MCprep 4.0. Once those are done, we'd also
need to discuss how this would coexist with the existing generation
based system (should we choose to keep it).

Closes #274

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations StandingPadAnimations added this to the v4.0.0 milestone Jul 7, 2024
@StandingPadAnimations StandingPadAnimations self-assigned this Jul 7, 2024
@StandingPadAnimations StandingPadAnimations linked an issue Jul 7, 2024 that may be closed by this pull request
@StandingPadAnimations
Copy link
Collaborator Author

Just to be clear as well, after MCprep 3.6 releases, I'll be taking a break from MCprep development to focus more on college applications and BpyBuild, so @TheDuckCow if you want, you can develop on this branch without conflicts for the most part. This is more of a long term thing anyway.

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

StandingPadAnimations commented Jul 7, 2024

After a little effort (mostly with missing stuff), I've added the ability to build MCprep with Vivy exposed in the UI through the enable-vivy-system action. This simply flips a global variable to be True instead of False. Note that there's no translation strings for Vivy yet, since the UI is subject to change.

Make sure when enabling Vivy support, you have vivy_materials.blend in MCprep_resources. I've also attached an existing Vivy export that can be imported through the UI.

vivy_export-2023-10-30.zip

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

Updated some of the references so basic things work. Vivy-based textureswap seems to not want to work for some reason, but prepping materials in general works fine

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

Got the Vivy-based textureswap fixed partially (issue was due to an outdated check for None instead of MCprepError)

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

Fixed the last remaining issue with Vivy-based textureswap, so now it should work on all texture packs

@StandingPadAnimations StandingPadAnimations linked an issue Jul 8, 2024 that may be closed by this pull request
1 task
@StandingPadAnimations
Copy link
Collaborator Author

Alright, got an idea to make merging this PR less of a pain: what if we make this PR solely dedicated to cleaning up and properly upstreaming the Vivy code (for say MCprep 3.7), and then gate the loading of Vivy behind a runtime check (which we do for the UI, but not the actual registration)? This does mean the UI is technically in an incomplete when merged, but it also means less of a pain with merge conflicts as the PR wouldn't be too far behind.

We could also make Vivy usable to end users using the experimental flag in MCprep, so we can get some community feedback (especially when we do start working on the UI).

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

StandingPadAnimations commented Aug 12, 2024

@TheDuckCow ready for first review. To be clear, this is a long term feature that will be developed over multiple PRs, and perhaps won't be enabled for users until MCprep 4.0. This PR specifically works on cleaning up the code for the Vivy material system, and getting it up to par with upstream standards. That way, we don't have to worry about a single long PR that gains conflicts over time, and instead have smaller PRs merged over a longer period of time. It does mean having unstable code in production, but I think that's a reasonable tradeoff to make.

By default, Vivy modules won't even be registered unless conf.ENABLE_VIVY is set to True, which can be enabled with the new enable-vivy-system action.

@StandingPadAnimations StandingPadAnimations marked this pull request as ready for review August 12, 2024 14:13
@StandingPadAnimations StandingPadAnimations modified the milestones: v4.0.0, 3.6.2 Aug 12, 2024
@TheDuckCow
Copy link
Member

Hey @StandingPadAnimations thanks for making a great head start on this! To make things easier, would you be able to record a short demo video of how it's mean to be used right now? That way you save some time documenting and it's easier for me to see how it's currently working. I installed the branch to see that at the very least, nothing appears broken per-se, but wasn't too sure where to go after there (without diving more into the code first).

We can either treat this as a feature branch, or try to merge parts of it at a time until the UI side is fully ready.

@StandingPadAnimations
Copy link
Collaborator Author

Here's some demos I uploaded on Mastodon a long time back (not much has changed though):

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

I've implemented refinements in the UI now, although mappings are not yet updated (so the UI won't show that a material is a refinement of X for instance)

@StandingPadAnimations
Copy link
Collaborator Author

StandingPadAnimations commented Feb 13, 2025

Testing the resource pack inconsistencies at the moment. Here's the texture packs I have tested with and their passing status:

  • Vanilla PBR 1-2-0
  • HardTop PiXXL Accurate 64x v0.4
  • Faithful 32x July 2024 Pre-release with PBR extension

These packs are definitely outdated a bit, but that's what I currently have locally. Will test some more based on suggestions.

@StandingPadAnimations
Copy link
Collaborator Author

So weirdly it seems to be that the errors occur if swap textures has been called once. This is likely due to fallbacks that occur the first time interfering with later calls

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

Subsequent uses of Vivy's texture swap has been fixed, albeit with a band-aid solution. Since a lot of the code feels rough, I'm going to spend some time simplifying it as much as possible

@TheDuckCow
Copy link
Member

Ok thanks @StandingPadAnimations - if you want, consider whether you want to split this up into several PRs or keep this one single one here. We could always create a feature branch. At some point, when it's ready, you and I should sit down and do a live review so I can make sure I grasp all of the current state of it and how it's working. Just let me know (via ping and/or request review)

@StandingPadAnimations
Copy link
Collaborator Author

Ok thanks @StandingPadAnimations - if you want, consider whether you want to split this up into several PRs or keep this one single one here. We could always create a feature branch.

The plan is to have several PRs for Vivy, with this one primarily focusing on the initial upstreaming (getting major bugs ironed out + cleaning up the code to meet existing standards). I think it should be fine to merge this into dev since right now, Vivy is an opt-in feature (taking advantage of the experimental option MCprep has)

At some point, when it's ready, you and I should sit down and do a live review so I can make sure I grasp all of the current state of it and how it's working. Just let me know (via ping and/or request review)

Sure thing!

@StandingPadAnimations
Copy link
Collaborator Author

Pushed refactoring changes and pulled in the latest updates to dev. @TheDuckCow I think this is ready for a live review

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@TheDuckCow
Copy link
Member

TheDuckCow commented Mar 23, 2025

I'm just installing the branch to prepare myself at least a little bit more for tomorrow, I notice a startup registration error - should probably just skip if we are in that state (didn't read too much further into it yet though, just flagging it) - restrict data typically is happening during startup but can also occur when accessing data from threads. In this case, seems to just be during the startup registration process if Blender is still working on registering (even if MCprep's own registration is still in progress, other addons might still be in progress - something I've run into recently on another project).

Traceback (most recent call last):
  File "/Applications/Blender 4.4/blender.app/Contents/Resources/4.4/scripts/modules/addon_utils.py", line 515, in enable
    mod.register()
  File "/Users/patrickcrawford/Library/Application Support/Blender/4.4/scripts/addons/MCprep_addon/__init__.py", line 65, in register
    load_modules.register(bl_info)
  File "/Users/patrickcrawford/Library/Application Support/Blender/4.4/scripts/addons/MCprep_addon/load_modules.py", line 192, in register
    mod.register()
  File "/Users/patrickcrawford/Library/Application Support/Blender/4.4/scripts/addons/MCprep_addon/mcprep_ui.py", line 2132, in register
    path = Path(bpy.path.abspath(addon_prefs.exp_vivy_file_path))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Applications/Blender 4.4/blender.app/Contents/Resources/4.4/scripts/modules/bpy/path.py", line 73, in abspath
    _os.path.dirname(_bpy.data.filepath)
                     ^^^^^^^^^^^^^^^^^^
AttributeError: '_RestrictData' object has no attribute 'filepath'

Also when trying to press "Edit vivy material library" I do run into this error, even after trying to save a file adjacent to the current one with that name. Hopefully we can run through it live tomorrow, patching as needed. We can also talk about this workflow tomorrow, the best way to suggest creating this and where it could be loaded from.

Screen Shot 2025-03-22 at 10 23 37 PM

Copy link
Member

@TheDuckCow TheDuckCow left a comment

Choose a reason for hiding this comment

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

As discussed, I'll be tagged for re-review once the small little hiccups we ran into today are resolved, and I'll do more of a deep review then for the scope as-is (I'll create a new issue for the follow up UX discussions around this, while the scope of this PR will still be in the experimental feature set)

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

@TheDuckCow Gotten all the bugs encountered during our meeting yesterday fixed, and the general editing pipeline seems to work on my end now.

All that's left are a few minor things (like changing Specular to Specular/Pack Map in the UI, and maybe some documentation fixes), but otherwise this is ready for a code review

@StandingPadAnimations
Copy link
Collaborator Author

I'll also look into the _RestrictData issue in the test suite

Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
Signed-off-by: Mahid Sheikh <mahid@standingpad.org>
@StandingPadAnimations
Copy link
Collaborator Author

Fixed the _RestrictData issue by using an app handler (that runs on a file load) to defer when the Vivy folder path is sourced from addon preferences. That way, by the time it runs, Blender should be ready to handle it.

I also fixed the test suite, it was downloading MCprep 3.5.3 for sourcing assets as opposed to the latest release

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

Successfully merging this pull request may close these issues.

LabPBR Resource Pack Support/Improvement Revamp Materials System
2 participants