Commit Graph

69 Commits

Author SHA1 Message Date
santorac b67862a133 Minor updates from code review feedback.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-11 09:37:12 -08:00
santorac 214b53de73 Fixed a unit test.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-09 16:22:44 -08:00
santorac 2a3c5b38e5 Added a unit test demonstrating that MaterialSourceData can support properties and subgroups together in the same property group.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-08 14:06:53 -08:00
santorac 8eafb7e057 Merge branch 'Atom/santorac/RemixableMaterialTypes4_FlatMaterialFiles' into Atom/santorac/RemixableMaterialTypes4_Layering
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-08 12:42:15 -08:00
santorac ddab03d678 Reponse to code review feedback. The main change is making GetPropertyValues return by const ref.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-08 11:18:17 -08:00
santorac 1daa9fbbed Changed the .material file format to use a flat list for material property values, instead of a tree structure.
This is needed to support deeply nested material property groups, it just makes the serialization code a lot simpler than trying to support nested groups in the .material file. It also makes the file more readable and easier to search all files for particular properties.
I also updated MaterialSourceData to hide the property values behind a clean API.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-04 12:04:50 -08:00
santorac 4bb93e4c94 Removed unnecessary MaterialSourceData::Property class.
This wrapper used to be needed in order to bind the custom JsonMaterialPropertyValueSerializer back when property values were represented by AZStd::any. Now that property values have a custom MaterialPropertyValue type, the wrapper class is no longer needed.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-03 14:58:03 -08:00
santorac 466e388f60 Fixed a unit test that broke in my last commit.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-03 10:28:43 -08:00
santorac a7bdb05d66 Tied up some loose ends with nested property group support.
Fixed the MaterialBuilder to report source dependencies on imported JSON files, so the material type will rebuild when you edit an imported file that contains shared property data.
Fixed a spot in LuaMaterialFunctorSourceData where it was only applying the old "prefix" feature and not the new name context.
Updated a couple built-in material functors to take advantage of the name context (they were using RuntimeContext::GetShaderResourceGroupLayout instead of the RuntimeContext::FindShaderInputConstantIndex wrapper utility function).
Fixed an issue with EnumeratePropertyGroups where it wasn't passing the right name context, so Material Editor wasn't able to load some material types.
Reordered the parameters in the MaterialTypeSourceData enumerate callback functions, I felt this order was more natural.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-01 22:37:47 -08:00
santorac 2c59f1b8a4 Added support for deeply nested material property groups.
The main addition here is the MaterialNameContext class which represents the concept of a namespace for properties, shader options, and SRG fields. This concept was already somewhat supported in LuaMaterialFunctor through bespoke "prefix" fields, but I have generalized it be available for all material functors. Note that I have not yet updated the other material functor types to ensure they take advantage of this feature, that will be in another commit.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-01 16:35:42 -08:00
santorac b1cc469914 Merge branch 'Atom/santorac/ShaderOptionValueUtils' into Atom/santorac/RemixableMaterialTypes4_Layering
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-01 12:24:50 -08:00
santorac 3441c02637 Minor cleanup of some shader option related unit tests.
Added new utility functions for easily creating the value set for a ShaderOptionDescriptor.
Made ShaderOptionDescriptor default value optional, picking the first available value as the default ... by default.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-02-01 11:42:50 -08:00
santorac d9c646062b Instrumented support for the JSON importer for material type files.
I also noticed that JsonFileLoadContext was no longer used (see https://github.com/o3de/o3de/pull/7010) so I was able to remove all that code.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-28 13:45:58 -08:00
santorac c2e220ce49 Renamed property 'set' to property 'group' for consistency with the prior naming.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-27 16:11:26 -08:00
santorac 906db920d0 Merge remote-tracking branch 'upstream/development' into Atom/santorac/RemixableMaterialTypes3
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-27 12:58:22 -08:00
santorac e0cee13747 Code cleanup.
Made PropertyDefinition::m_name private.
Moved code around for a cleaner diff in MaterialTypeSourceData.h.
Added API comments.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-26 18:10:02 -08:00
lumberyard-employee-dm b9824ed172 Updated all array_view uses with the C++20 span. (#7157)
* Updated all array_view uses with the C++20 span.

The updates were done in the following order
1. `AZStd::array_view<([^>].+)\* ?>`  -> `AZStd::span<\1 const>`
2. `AZStd::array_view<(?:const )(.+)>` -> `AZStd::span<const \1>`
3. `AZStd::array_view` -> `AZStd::span`

Removed the implementation of array_view.

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>

* Added missing whitespace between `const` and the typename for spans.

Updated the ShaderTest comparison of the ShaderResourceGroupLayout span
to compare the sizes as well

Updated comments on some of the methods that stated that they return "an
array" to mention they return "a span".

Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com>
2022-01-26 16:15:47 -06:00
santorac 4312c636af Got the RPI unit tests building and working again after merge. There were some incorrectly resolved conflicts that I had to re-resolve, especially in MaterialTypeSourceData::CreateMaterialTypeAsset.
I removed support for property rename version updates in MaterialTypeSourceData (i.e. ApplyPropertyRenames) because MaterialSourceData serialization no longer loads material property definitions from the .materialtype file, per a recent change on the development branch. The unit tests were broken and it wasn't worth updating them since we don't need this functionality anymore. Material property renames and other version updates are now exclusively applied by the MaterialAsset class.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-26 12:19:49 -08:00
santorac b5c7869f14 Merge branch 'development' into Atom/santorac/RemixableMaterialTypes3
There were lots of material system conflicts that had to be resolved. I expect the build is broken at this commit, and I'll fix it in followup commits.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-25 14:47:24 -08:00
santorac b9ba9f5ca8 Got RPI unit tests building and passing again.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-20 16:11:31 -08:00
santorac 02acf2f65e Updated MaterialPropertyId class in preparation for nested material property sets.
Here the class has been generalized for a list of group names and a final property name, rather than assuming a single group containing the property. This included removing the unused GetPropertyName and GetGroupName functions. All that's really need from this class is conversion to a full property ID string.

Testing:
New unit test.
Reprocessed all core material types and StandardPBR test materials used in Atom Sample Viewer's material screenshot test.
Atom Sample Viewer material screenshot test script.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-20 15:43:22 -08:00
santorac a896ff11bc Changed .material serialization to avoid loading the .materialtype file, since the .material builder doesn't declare a source dependency on the .materialtype. Otherwise there can be ambiguous edge cases where changes to the .materialtype might or might not impact the baked MaterialAsset. Note that another option would have been to add a the appropriate source dependency, but that would hurt iteration time as any change to the .materialtype file would cause every .material file and .fbx to rebuild.
These changes have the added benefit of simplifying some of the serialization code. MaterialSourceDataSerializer is no longer needed, as its main purpose was to pass the MaterialTypeSourceData down to the MaterialPropertyValueSerializer.

Before, the JSON serialization system gave a lot of data flexibility because it did best-effort conversions, like allowing a float to be loaded as an int for example. But now the material serialization code doesn't know target data type, so it has to assume the data type based on what's in the .material file, and then the MaterialAsset will convert the data to the appropriate type later when Finalize() is called.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-18 19:00:50 -08:00
santorac 45429872d6 Switched back to making MaterialAsset::GetPropertyValues automatically finalize the material asset. I realized that it's too burdensome to expect client code to call Finalize on the MaterialAsset; every code that calls GetPropertyValues would have to call Finalize(). Instead of using const_cast in GetPropertyValues like I was doing before, I just changed GetPropertyValues to be a non-const function. There were a few places in Decal code I had to update to pass non-const MaterialAsset pointers. This isn't ideal, but I think it's better than the alternatives.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-18 17:39:44 -08:00
santorac 2d6d14abf7 Changed MaterialAsset::GetPropertyValues to not auto-finalize. Client code must call Finalize manually. It's better to avoid unexpected side-effects from a const getter function.
In some cases it may be acceptable to do non-const things in a const function as long as it is only manipulating internal data, and the public facing API returns the same values as before. But in this case, the IsFinalized function is a public facing API that would have a different result after GetPropertyValues was called.

I also updated a couple other minor things from code review feedback.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-18 10:29:56 -08:00
santorac 1fa1eaad15 Added unit tests for the new functionality.
I found a mistake where MaterialAssetCreator needs to clear the raw data when configured to finalize the material asset.
Since MaterialSourceData no longer relies on the material type source file at all, I was able to change MaterialSourceDataTest to avoid saving the source data to disk.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-18 10:29:49 -08:00
santorac a627cda5ae Got the unit tests working again.
I made MaterialAsset::Finalize private so I could add some parameters specifically for MaterialAssetCreator to use. Now MaterialAssetCreator::Begin has an option to finalize the material or not.
Moved MaterialAssetCreatorCommon::ValidateDataType to MaterialPropertyDescriptor as "ValidateMaterialPropertyDataType" so that MaterialAsset::Finalize could use it too

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2022-01-18 10:29:48 -08:00
Steve Pham 4670003aaf Fix failed Atom rpi tests linux (#6319)
- Remove trait AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
- Add template specialization for ConstantsData::SetConstant<Color> and ConstantsData::GetConstant<Color>
- Remove trait AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS from source code
- Fix ConstantsData::SetConstantArray<bool> to pass unit test SetConstant_GetConstant_FalsePackedInGarbage_Bool on Linux, cause by an unwanted optimization (comments describe the situation). This will make this behavior consistent across platforms

Signed-off-by: Steve Pham <82231385+spham-amzn@users.noreply.github.com>
2021-12-10 14:54:08 -08:00
santorac 9a887bd690 Removed unused lambda function.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-11-17 15:11:05 -08:00
santorac d3de1689ad Removed unused lambda function.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-11-17 13:53:11 -08:00
santorac 2ae7a5ef9f Added a unit test for the MaterialPropertyValue bug fix.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-11-17 12:04:46 -08:00
santorac 7d48e8209c Updated a failing unit test.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-11-16 17:28:57 -08:00
santorac 0f60d37fec Fixed a bug where material version updates didn't support moving a property from one group to another.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-25 13:32:32 -07:00
santorac 3c331e00ff Moved the Material Component property override renaming to EditorMaterialComponent via ApplyAutomaticPropertyUpdates.
MaterialAssignment::ApplyProperties() still reports warnings but does not update the m_propertyOverrides.
MaterialAssignment::ApplyProperties() will now skip the old name'd overrides if overrides are present for the new names. I'm not sure if this will ever happen, but it did happen while I had some intermediate changes, so I imagine it could happen again.
I had to update the Material::FindPropertyIndex function to expose information about renames when they occur. This should make it easier for other systems to get (somewhat) automatic benefit from the version update feature.
I also found that there was an issue with material inspector where it wouldn't be initialized the the right override values when renames were present. Now it applies the renames to whatever override data it gets from the Material Component.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-22 16:41:44 -07:00
santorac c6b209ace0 Made material property auto-rename procedure apply to Material Component at runtime. This ensures that an material property overrides and any gameplay scripts that work with property overrides can get the benefit of the material type version update procedure.
I added an ApplyPropertyRenames function to MaterialTypeAsset very similar to the one in MaterialTypeSourceData.
Updated the MaterialAssignment class to apply any property renames when it discovers the old name doesn't work. This will be written to disk when the level or prefab is saved.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-21 23:18:08 -07:00
santorac f4d5a75574 Fixed an issue where the wrong version number was being reported in a warning message. It showed the latest version instead of the original version number.
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-21 12:13:45 -07:00
santorac f83b6594c9 Updated MaterialTypeSourceData to force users to move the "version" indicator to the new location at the top level of the json document.
(It's a simple enough change to make manually, and making .materialtype is an uncommon workflow, so not worth doing this automatically).

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-21 11:05:47 -07:00
santorac 46061eb302 Simplified the code around MaterialAsset::ApplyVersionUpdates()
Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-21 00:07:36 -07:00
santorac 1633ced656 Reports warnings when a material version auto update is applied, notifying the user they should update their source data.
Also improved the MaterialAssetTests UpgradeMaterialAsset() to focus on testing the inputs and outputs of the class rather than the private internal data.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-20 23:36:53 -07:00
santorac 836d018de7 Simplified MaterialVersionUpdate code to focus just on renaming, since that's the only operation we currently support. We shouldn't add more complexity until additional operations need to be supported.
Also rearranged some logic to simplify the code that loops over rename actions, avoiding making unnecessary additional maps.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-20 15:53:22 -07:00
santorac a0b1dec929 Added support for material version updates in MaterialSourceData. This is necessary for tools like Material Editor and Asset Processor to work with the latest property names.
- Added MaterialSourceData::ApplyVersionUpdates() for updating the properties. This should be called by tools after loading the MaterialSourceData. (But can be omitted if a tool wants to read the data exactly as it appears in the .material file).
- Updated MaterialTypeSourceData::FindProperty to support applying version update renames, including a ApplyPropertyRenames utility function, which are necessary for MaterialSourceData to be able to find the necessary property definitons while loading.
- Added a new context struct to JsonMaterialPropertyValueSerializer for passing down the material type version number, to help with applying property renames.
- Renamed the .material file format "propertyLayoutVersion" to "materialTypeVersion" which is more accurate. This shouldn't hurt existing data as this field wasn't actually used for anything before.
- Updated Material Editor to again store the material type version number in .material files.

MaterialSourceDataTests updates...
- Updated to include both a .materialtype file and a MaterialTypeAsset for the test material type. Both are used by the MaterialTypeSourceData class.
- The default test material type now includes some version update steps; these are only used for version update tests and won't impact the other test functions.
- Updated the path for storing temp files to disk, to just be in a "temp" folder in the exe path. (Originally they were saved to the gem folder near MaterialSourceDataTests.cpp, but at some point someone changed it to be under the exe folder, so there's no reason to use the full gem path anymore).

MaterialTypeSourceDataTests updates...
- Moved some code that was accidentally added to LoadAllFieldsUsingOldFormat but should have been in LoadAndStoreJson_AllFields.
- Added test cases for unsupported version update operations

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-20 15:31:01 -07:00
hershey5045 d8549f9566 Merge branch 'development' into Atom/rbarrand/MaterialVersionUpdate 2021-10-05 11:39:27 -07:00
santorac b1c7469687 Renamed m_groups and m_properties to have "Old" in the name for clarity.
Also fixed a potential uninitialized data bug in Conve.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-10-01 17:11:54 -07:00
santorac 1a99103999 Overhauled the .materialtype file format to group all related properties data together. This prepares the way for a number of possible improvements, especially unlocking the ability to factor out material type configuration to be shared by multiple material types.
Here we formalize the concept of a Property Set, which replaces property "groups", containing the group name and description, properties, and functors all in one place.
The Property Set structure will allow arbitrarily deep nesting, whereas before you only had one level of grouping. This nesting is not fully supported yet throughout the system, particularly in the Material Editor. It was easier to go ahead and put in some of the nesting mechanims, parituclar in the implementation of MaterialTypeSourceData.
This change is backward compatible, which is proved with unit tests, and by the fact that only MinimalPBR.materialtype has been updated to the new format. StandardPBR, EnhancedPBR, and others are still using the old format. (In a subsequent commit I'll update these as well, to prove that the new format works correctly).

Other changes and improvements...
- A new constructor for MaterialPropertyId
- Improved API for MaterialTypeSourceData that hides a good deal more of it's data as private, with clear and convenient APIs. Especially AddProperty, AddPropertySet, FindProperty, FindPropertySet, EnumerateProperties, EnumeratePropertySets.
- Added lots of new unit tests
- Updated MinimalPBR.materialtype to the new format.

Testing:
- Updated unit tests.
- Reprocessed Atom material assets.
- Ran AtomSampleViewer material screenshot test.
- Opened, edited, saved material in the Material Editor.
- Opened a level, edited material property overrides, saved and reloaded.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-09-30 01:30:28 -07:00
santorac a7c1bf2ec5 Merge branch 'Atom/santorac/RemixableMaterialTypes' into Atom/santorac/RemixableMaterialTypes2 2021-09-28 22:14:23 -07:00
santorac c8d2f74ca1 Added backward-compatible support for the old "id" key in material type files, which is being renamed to "name".
This required the use of custom serializers, because the JSON serialization system does not have any means of supporting field name aliases through SerializeContext.

Testing: RPI unit test pass and AtomSampleViewer material screenshot test script passes.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-09-28 22:05:26 -07:00
Robin ca06e2e82d Update reflection version markers.
Signed-off-by: Robin <rbarrand@amazon.com>
2021-09-27 23:44:13 -07:00
Robin 2607b3471a Resolve PR comments. Add unit tests.
Signed-off-by: Robin <rbarrand@amazon.com>
2021-09-27 23:30:59 -07:00
santorac 55eb85d546 Updated MaterialPropertyId class in preparation for nested material property sets.
Here the class has been generalized for a list of group names and a final property name, rather than assuming a single group containing the property. This included removing the unused GetPropertyName and GetGroupName functions. All that's really need from this class is conversion to a full property ID string.

Testing:
New unit test.
Reprocessed all core material types and StandardPBR test materials used in Atom Sample Viewer's material screenshot test.
Atom Sample Viewer material screenshot test script.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-09-18 23:57:52 -07:00
santorac 71c7fc0217 Updated unit tests to work with the new convention for property name vs property ID.
This revealed that the approach of reflecting both the old "id" and the new "name" would not work, because whenn saving it would write out both fields. So I decied to just give up on backward compatibility. This will be much cleaner than trying to continue supporting "id" as a field name, it is uncommon for users to make their own material types at this point, and if they have made some it is very easy to search and replace "id" with "name" update their files.

All .materialtype files have been updated. RPI unit tests now pass. ASV still passes.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-09-17 23:12:24 -07:00
santorac c2abd2d74f Updated the naming convention for material property "names" vs "IDs".
A "property name" is the name of the just the property without regard to the group that it's in. A "group name" is the name of the group. And a "property ID" is the full unique name of a property in the form "groupName.propertyName". This is important preparation for upcoming changes where property sets can contain other property sets, and property IDs can be arbitrarily long like "layer1.baseColor.factor" for example.

The naming changes include variables, some code comments, and the .materialtype file format. I was able to make these changes in a backward compatible way so a property or group "id" field has been replaced with a "name" field, but "id" is still supported for compatibility. StandardPBR, EnhancedPBR, StandardMultilayerPBR, and Skin have all been updated. Note that MinimalPBR has not been updated, proving that backward compatibility works. (We can update this one too at some point though).

Testing:
Opened up materials in the material editor.
Ran AtomSampleViewer in dx12 and vulkan with no new failures.

Signed-off-by: santorac <55155825+santorac@users.noreply.github.com>
2021-09-17 22:19:31 -07:00