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

Gadams3/cherry picking material fixes from development into point release 23101 #17069

Conversation

gadams3
Copy link
Contributor

@gadams3 gadams3 commented Nov 15, 2023

What does this PR do?

Cherry picks several material and shader related fixes from the development branch into point released 23101 from the following pull requests.

#16836
#16837
#16838
#16840
#16871
#17025
#17035
#17038
#17050
#17055

  • Fixes several warning and error messages when processing materials and shaders.
  • Fixes several problems with material, material type, and shader builders.
  • Fixes material, material type, and shader asset dependencies to better support hot reloading when making changes to any of the above.
  • Assets should process in order and shaders and materials should hot reload reliably unless several changes are saved in rapid succession, which may prevent reliable ordering with several related jobs already being processed.
  • Changes instance id implicit constructors to explicit create functions with support from multiple sub ids In support of hot reloading and substituting instances with newer asset versions.
  • Updates systems using instance database and instance ids to use the new functions.
  • Changes to the asset browser to process change set updates on the system tick bus so that it does not lock up when switching between applications.
  • Changing the auto generated all properties material to use the JSON extension and have a custom asset type id so it doesn't create spam while starting the editor or be confused with a proper material.
  • Fixed material component generate source materials to reference the originating source material instead of the generated source material in their intermediate assets holder.
  • Fix this shader resource group pool instances to ensure they are unique by considering the SRG hash.

How was this PR tested?

Ran asset profile build locally to build all assets for server and PC platforms.
Smoke tests running editor, material editor, and using material component.
Discovered unrelated issues with asset picking from the inspector with dpe enabled.

While using material canvas or manually editing shaders, modifying existing assets, the root asset browser entry class consistently asserts about missing or already registered files and folders. I can only assume that this is going unnoticed either because of different behavior in the main editor, material canvas repeatedly saving over the same files, or because asserts were previously non fatal. This makes me question if asserts should remain non fatal in stabilization and release.

Signed-off-by: gadams3 <guthadam@amazon.com>
loading shader templates into shader source data structure instead of raw text buffer and performing search and replace

using file IO path and changing all generated paths to posix with forward slashes

addressing problems when referenced files start with U, as in untitled, which the shader compiler interpreted as Unicode escaping

changing several container adds from push to emplace

Signed-off-by: gadams3 <guthadam@amazon.com>
• This change switches existing standard and base PBR material graph nodes and templates from defining explicit shader and azsl files to instead use the material pipeline. This will significantly reduce the number of files that need to be directly output from material canvas and managed in source control by users. It also means that generated materials and shaders will behave more consistently with their hard coded counterparts and work with any future, compatible material pipelines, including mobile. This should also resolve any issues with inconsistent behavior with transparency.
• Along with this, a change was made to the material graph compiler to update the fingerprint on generated assets so that they get reprocessed by the AP and notifications are sent even if in some files, like the material file, are unmodified. Material source files generated by material canvas typically never change after their first generated because they only referenced the generated material type.
• Bounds checking was added to getter functions on the shader input constant layout glass. There was a potential crash while over indexing a container while interim shaders or shader variants are reloaded as part of a material that has not yet been fully reprocessed.
• Minor updates to asset status reporter messaging and sleeps.

Signed-off-by: gadams3 <guthadam@amazon.com>
Signed-off-by: gadams3 <guthadam@amazon.com>
Changes started as updating shader load error messages to distinguish between asset load failure and instance creation failure.

Corrected multiple checks for valid asset ids or asset pointers that should have been checking for the asset to be ready.

Changed strings to constexpr const char*.

Signed-off-by: gadams3 <guthadam@amazon.com>
- Changed instance ID sub ID and version tracking. Instance ID now contains a fixed vector of sub IDs that is used consistently when the instance ID is created from an asset or an asset ID. Additional sub IDs can be added to an instance ID, regardless of how it is created, for custom unique values. Shader and shader resource group pool instance IDs are now mapped directly to the same instance ID layout without packing and hashing a structure into a UUID.
- Completely removed spam warning messages from root asset browser entry class. The warning messages, which were previously asserts, indicate that files or folders have been updated or already exist in the asset browser model.
- Updated material and material type builder source dependency so root materials and material types include dependencies for top level intermediate source assets.
- Updated graph compiler to not clear graph or other members during state changes in case data was still being used or read during transition.
- Made material canvas asset fingerprint resetting optional put on by default so that additional testing could easily be done with it enabled or disabled.

Material canvas generates source files that get reprocessed by the AP and potentially hot reloaded in the tools. The data produced by material canvas goes through the same processing and systems that hand authored materials and shaders do. Underlying systems that independently monitor and reload shaders and shader variants can trigger asserts if updated before all of the related materials and shaders have updated to the same version. The ideal solution is to completely remove shader reload notification bus and have concerns systems listen for regular asset system reload notifications for shaders and shader variants.

Signed-off-by: gadams3 <guthadam@amazon.com>

Adding asset ready handler to editor material component slot

Signed-off-by: gadams3 <guthadam@amazon.com>

Removing self reloading from material class

Signed-off-by: gadams3 <guthadam@amazon.com>

updated instance ID struct to replace the single sub ID and version ID with a vector supporting multiple sub ID's
updated create from asset and create from asset ID functions to use the same layout with the asset ID sub ID and creation token with a default value

Signed-off-by: gadams3 <guthadam@amazon.com>

Replaced shader and shader resource group pool instance ID generation with new instance ID supporting multiple sub IDs

Signed-off-by: gadams3 <guthadam@amazon.com>

Fixing errors related to instance ID changes

Signed-off-by: gadams3 <guthadam@amazon.com>

Completely removing spam messages from root asset browser entry

Signed-off-by: gadams3 <guthadam@amazon.com>

Changing instance ID sub ID container to fixed vector and adding comments

Signed-off-by: gadams3 <guthadam@amazon.com>

Using the hash as sub IDs for the shader resource group pool instance IDs

Signed-off-by: gadams3 <guthadam@amazon.com>

Making clear fingerprint optional but on by default

Signed-off-by: gadams3 <guthadam@amazon.com>

Only clearing graph pointer on new compile request

Signed-off-by: gadams3 <guthadam@amazon.com>

Rename fingerprint setting

Signed-off-by: gadams3 <guthadam@amazon.com>

updating comments

Signed-off-by: gadams3 <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
… creating instance IDs from asset IDs

Signed-off-by: gadams3 <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…tainers

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…type instead of bool

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…to binary, and updating builder versions

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…n incomplete or redundant paths

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…type builder

Consistent loading for material and material type source data, which was done in different ways and multiple code paths
Recording fingerprints when dependencies are added for materials or material types

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…pendency configuration

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
\If the changes are made directly in explorer, notepad, DCC tools, or another tool like material canvas that take focus away from the application hosting the asset browser then all of the file and folder changes would be accumulated and only applied once the application main window regained focus. This could freeze the editor for several seconds while the asset browser updated and synchronized to apply the changes.
Switching to system tick allows the change to be applied in the background over time.
This issue was causing problems with verifying that has sent notifications were triggered and received by the editor for external changes

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…nd relative path warnings

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
… mapping only uses the UUID

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
Removing tracing from instance database.
Change material graph compiler asserts into errors.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: ANT/guthadam <guthadam@amazon.com>
… cannot be found so that assets will be reprocessed once they rip here, usually in an intermediate assets

Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…server builds

Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…her than PC

Cleaning up some asset utilities

Signed-off-by: ANT/guthadam <guthadam@amazon.com>
…material type being used to create a material asset.

Other minor cleanup.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…th because it was already resolved to load the material type source data.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…lete paths.

Updated usage of get possible dependency paths to handle wild cards when setting up dependencies.
Removed common platform identifier from abstract material type dependencies. This allows the assets to compile with server and pc running simultaneously, resolving everything correctly. However, it produces more warnings about unresolved dependencies. These warnings were present prior to the previous material builder and dependency changes.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…dependency for materials

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…o be explicit and added implicit wild card shader dependencies to prevent material builder from processing before shaders are complete

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…d wild card job dependencies.

Added dependencies on shaders to material builder so the effect the fingerprint and force the material builder to wait on them to rebuild, even though this should be handled by the job dependency on the material type itself.
Added utility function to material type source data to return a vector of shaders from all of the collections.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…ns to instead directly add source or job dependencies as needed.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…ueued during notifications

This addresses a case where a user was setting property values during notifications for materials being created and updated. These notifications are sent after property values for the current tick are applied. Changes made within the notifications would not get applied until other work was queued at a later time.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>

corrected typo after review feedback

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…erial

The material type builder saves a default material source file as a convenience for a technical artist to be able to enumerate all of the material properties without parsing the material type file. File was originally saved with the material extension which caused warnings whenever opening the main editor. This change renames the extension to JSON so the file is not recognized as a material source file to stop the warnings.

Resolves o3de#13677

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…SOM file products

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…dependencies and source dependents

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…asset material type file path back into the originating material type file path.

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…urce material instead of the intermediate asset version

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…e to the original source material type if it was previously saved referencing a generated intermediate asset material type

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…ion back to original source path

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…ng to az io path in new changes

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
… data with mismatching ids

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
…rg fields

Signed-off-by: Guthrie Adams <guthadam@amazon.com>
@gadams3 gadams3 merged commit 69bacd1 into o3de:point-release/23101 Nov 15, 2023
3 checks passed
@gadams3 gadams3 deleted the gadams3/cherry_picking_material_fixes_23101 branch November 15, 2023 23:46
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

3 participants