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

New Texture and Shader Generator #127

Merged
merged 24 commits into from Jul 4, 2020
Merged

New Texture and Shader Generator #127

merged 24 commits into from Jul 4, 2020

Conversation

ghost
Copy link

@ghost ghost commented May 28, 2020

Overview

The material generators for the Cycles and Eevee rendering engines have been rewritten to support a variety of PBR pack formats and future shader node setups. Using the new structure, many more PBR formats and shader node setups can be added if needed.

New structure for node generation

The new structure for node generation splits the texture generating process into a sub-function of the shader generators.

For example, a shader generator function is run after a user uses Prep Materials. This will generate the necessary shader nodes for each material, such as principled, transparency, and emission. After this has completed, the shader generator function will then call a sub-function. This sub-function will be one of the texture generators. The shader generator will also pass a nested array to the texture generator, containing the sockets it must connect to in the shader nodes. The texture generator then uses this to properly set the image texture nodes and any conversion that must go after it, and finally connecting the outputs to the input sockets listed in the nested array.

Progress

Shader Generators

  • Principled
  • Non-Principled

Texture Generators

  • Specular
  • SEUS

Custom material generators

  • Glass
  • Water
  • Others???

Other

  • Implement the light falloff node again – I accidentally forgot to put it back

Compatibility checks

  • 2.83 - 2.82
  • 2.81 - 2.80
  • 2.79

Related Issues

Extra Notes

Sorry this took so long to get up, I had a username transition long in the works and didn't want to mess up the commit history. Also, Git LFS was not running on my Mac due to the Catalina code certificate requirements, and I spent an hour trying to find out why I couldn't make a simple commit(rip– got it fixed though)

A lot of stuff was moved around, deleted, and added, so I may have accidentally deleted a feature somewhere and forgot to put it back. I will need to check on this.

I made a lot of smaller commits, to make changes easier to see. Looking at the commits on master, the commits in this pull request should probably be squashed before merging to stay consistent.

Some things may have been done in an inefficient matter due to my limited understanding of the whole structure of this add-on. I will need to check on this.

Using Eevee on a Mac with these generators will basically freeze Blender. This was an issue with other versions of MCprep, as it is a Blender issue(Unknown cause at the moment).

Input and output references should be changed to a string, instead of an integer value, to improve consistency across versions.

There are probably bugs and unclean code everywhere, as I am not 100% fluent in Python. Also, I may have left some different formatting styles than what is used here, I will need to change these.

bytzo added 8 commits May 27, 2020 09:48
Added file paths generated by Python and MCprep when building the add-on
Add UI options to select each Pack Format above the "Prep Materials" button. "Specular" is selected by default.
The logic to generate the materials has been added in the function matprep_cycles. A principled generator has been added for development functionality.

Note that the material logic and principled generator will not work fully without additional generators.
The specular shader has been added. Placeholders for the SEUS and labPBR formats have been added for future use.
The specular shader has been added. Placeholders for the SEUS and labPBR formats have been added for future use.

Fixes #122 in the Specular generator, however only tested in Blender 2.82a
The SEUS Generator has been added. The R, G, and B channels are now correctly seperated and used in the principled generator when using "Prep Materials."

Note at the time of commit, the non-principled shader has not been written, using the non-principled generator will revert to the old method.
Removed the emission generator. This has been replaced by use_emission attribute in the Cycles principled and non-principled generators.
@ghost ghost mentioned this pull request May 28, 2020
@TheDuckCow
Copy link
Member

Thanks so much for starting to work on this! I'll not rush to review since it's in draft, but let me know if you'd like comments earlier than later.

I didn't realize git lfs would cause any issues when using locally, I thought it was basically a passthrough and all repo-side managed. What did you end up having to do, for my references? In general, I should make it easier for people to contribute to this repository, so let me know any other experiences you have to share on this point.

For my understanding, the freezing is only when Eevee is selected, at the time of swapping textures right? ie Eevee loading the textures and compiling the shaders (even if you aren't yet in rendered mode); if something different let me know.

@ghost
Copy link
Author

ghost commented May 28, 2020

It seems that the Git LFS issue only is a problem with macOS 10.15 or greater. I had Git LFS installed, so Git was trying to use it to pull the large files. However, the Git LFS script does not have a signed certificate, so Apple blocks it because it considers it dangerous. This caused Git to think remote changes needed to be pulled, since none of the LFS processes correctly executed. It looks like this has been fixed as of yesterday, so on the next release of Git LFS this should not be an issue. As a workaround for now, I used $ sudo spctl --master-disable to bypass macOS certificate checks.

Regarding the Eevee issue, it is only an issue with Blender. The issue is that certain shader nodes will freeze Eevee in the viewport and render(macOS). However, this is extra strange behavior because more complex nodes do not cause the issue, and reproducing is inconsistent. To test, I created a Suzanne with 6 material slots and set them to random things, like gloss and emission. After changing the materials over and over with random nodes, things seem to stay smooth. However, once I add a gloss shader on one of the material slots, Eevee freezes up. Removing the gloss fixes the issue. Keeping the gloss shader on the specific material slot and removing another gloss shader on another slot does not fix the issue. Replacing the gloss with a more complex shader like a metallic principled fixes the issue. I have been able to reproduce this issue with other shader nodes, like emission and glass. This does not happen on Windows however.

It seems to be very random, but I noted it here since the MCprep generator seems to trigger this issue in the shader preview/rendered viewport for my machine after prep materials. Using MCprep in solid mode will not instantly trigger the issue, but switching to an Eevee viewport afterwards will.

This is not something we need to worry about though, as it is an issue with Blender.

@TheDuckCow
Copy link
Member

On your last point, agreed - and very familiar with the issue itself. You actually helped my understand and identify it a little better, but yes - being a Blender issue, not something to be addressed here.

Regarding the rest, let me know if you want me to try and do a first pass review as-is. I'm okay with having to correct some things after submission anyways (pointing out that merging will be done into the dev branch); do see if you can sync, I made some updates to the grayscale generator which I think improves it, so it might not have been your fault.

I'm also fine if we leave labPBR as a separate followup commit, in part to act as self-serving documentation of how to add more types in the future in a single commit (and not bringing in the other infra changes you are making here).

bytzo added 5 commits June 7, 2020 12:38
The location for the Pack Format option was moved to the Prep Materials popup, and some files were formatted.
Some materials which had specified emission maps were not displaying correctly due to the automatic removal of the emission node. This has been changed to always include the emission map if a specular map is present, otherwise use emission based on the block.
Added a special water generator for generating a water texture using the glass and transparent shaders.
@ghost
Copy link
Author

ghost commented Jun 17, 2020

Sorry for leaving this response for a while– I have been a little short on time.

A first pass-review would be great! However do note many things are still being worked on and some code may not be the most efficient, as I am not fluent with Python. If you would like to implement some changes now, you should be able to commit to bytzo:dev as "Edits by maintainers" is enabled. I've actually never used this feature, so I can't guarantee it works.

The grayscale changes in de53d3f were merged with 1b3dfce. Also, I agree with you about the labPBR part, so I have removed references to labPBR in f6f5f8b.

On another note, starting with commits 5dd61cf, 199b694, and eedf027, I used a new development environment, which includes a Python linter. Apparently, Python code should be indented with spaces, which means the linter changed all the tabs to spaces, and added random line breaks where the lines were too long. I hadn't realized this until almost done with 5dd61cf, and I had no easy way to undo the changes, as the linter works on save. Let me know if you are ok with this, or if I should rewrite the commits (these will be squashed, so shouldn't be different than reverting commits). The linter should be off now, so it shouldn't be an issue in the future. Sorry about this mess, I will be sure to check the diff editor more carefully in the future before staging changes.

Not very important, but the Eevee issue with macOS should be fixed now, see this. I believe this was included in the 2.83 LTS release.

Added a special glass generator for generating a glass texture using the glass and diffuse shaders.
@TheDuckCow
Copy link
Member

Hi there, technically spaces vs tabs are optional. Though spaces are typically preferred, the higher level convention is consistency with existing repo code. I can easily change it back to tabs later so don't sweat it too much right now, I can make that change when we're getting close to merging into the dev branch here.

I'll start adding some snippets of comments here and there, but don't let that interrupt you! Thanks again for the doing the good work here. And good to know the issue is resolved on OSX now, super helpful.

@ghost ghost mentioned this pull request Jun 29, 2020
Added back the light falloff node and cleaned up messy or unnecessary code.
A non-principled generator was added in addition with updated logic for special glass generators and linking nodes.
@ghost ghost marked this pull request as ready for review July 1, 2020 01:09
@ghost ghost requested a review from TheDuckCow July 1, 2020 07:19
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.

Thanks again! Very close I think.

I was able to test against at least one texture pack and seemed to work quite nicely. Main note I think is with the water, the nodes overlap a bit still, plus it's still applying a multiply of blue even when the source image is already saturated. I'm perfectly fine addressing both of those things in my own followup commit.

On the comments I added, let me know if you're able able to make those changed - if not, just let me know and I can take it from here!

MCprep_addon/materials/generate.py Outdated Show resolved Hide resolved
MCprep_addon/materials/generate.py Show resolved Hide resolved
MCprep_addon/materials/generate.py Outdated Show resolved Hide resolved
MCprep_addon/materials/generate.py Outdated Show resolved Hide resolved
MCprep_addon/materials/generate.py Outdated Show resolved Hide resolved
MCprep_addon/materials/generate.py Outdated Show resolved Hide resolved
@TheDuckCow
Copy link
Member

Also, couple more notes for my future self to implement (ie you don't have to address yet, let's get this PR in first):

  • Change the current dropdown which says "Specular, SEUS" to also have "No extra maps" (or maybe Single Texture? ie the equivalent to unticking "Use extra maps" currently present). Open debate to say which one should be the default, though down the line I'd love to do a better job at detecting. "No extra maps" or specular should still be the default for optimal render times, in my mind.
  • Add option to prep materials at the same time as swapping textures. With this pulll request, the paradigm is no longer in place that we can reuse the exact same node layout for everything. Thus, we provide the option (again, by way of dropdown) to regenerate materials and select the according method. This could be a dropdown in the side panel with options: Don't prep (ie currently behavior), No extra maps, Specular, SEUS

@ghost
Copy link
Author

ghost commented Jul 3, 2020

Thanks for the review! I've fixed most of the issues so far, including the misplaced water nodes and the extra color mix. Additionally, I've quickly implemented a prep material function when swapping texture packs. This has been included in the sidebar in the texture pack selection window. The options for prep mats will automatically disappear if "Prep materials" is disabled. "Prep materials" is set to false by default.
Screen Shot 2020-07-02 at 10 34 58 PM
I still haven't tested in 2.81 or 2.82, however things should work mostly as 2.79 seems to function fine. Again, thanks for reviewing this!

@TheDuckCow
Copy link
Member

Thanks so much for working on this already already implementing this! And, should have read this last comment first. Awesome work to get the re-drawing functional. Thanks for that!

I feel pretty confident we can merge now, is there anything else pending in your mind?

@ghost
Copy link
Author

ghost commented Jul 4, 2020

Did a few more quick tests with 2.79, and it seems good! I don't believe I have any more changes in mind, so you are all good to merge! Thanks for taking the time to review this :D

@TheDuckCow TheDuckCow merged commit 6167aa8 into Moo-Ack-Productions:dev Jul 4, 2020
@ghost ghost deleted the dev branch July 8, 2020 07:58
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.

None yet

2 participants