On Thu, 9 Nov 2023 02:44:44 GMT, Michael Strauß <mstra...@openjdk.org> wrote:
>> Moves the filter setting of the samplers from the device parameters >> configuration to the use-site, allowing for dynamic changes in the sampler. >> This PR does internal plumbing work only to bring it close to the ES2 >> pipeline. A followup PR will create the public API. >> >> Summary of the changes: >> * Created a new (internal for now) `TextureData` object that is intended to >> contain all the data of texture (map) of `PhongMaterial`, such as filters, >> addressing, wrapping mode, mipmaps etc. **This PR deals only with filters** >> as a starting point, more settings can be added later. >> * Creates an update mechanism from the Java side material to the native D3D >> layer. The public API `PhoneMaterial` is *not* changed yet. The peer >> `NGPhongMaterial` is configured to receive update from the public >> `PhongMaterial` when the public API is created via new >> `ObjectProperty<TextureData>` properties. >> * Small refactoring in the D3D layer with a new map types enum to control >> the texture settings more easily. >> >> The JBS issue lists some regressions in a comment, but I couldn't reproduce >> them. It looks like the sampler settings needed to be added anywhere, and >> that was the easiest to do at the time. Now they were just moved. > > modules/javafx.graphics/src/main/native-prism-d3d/D3DPhongMaterial.h line 41: > >> 39: self_illumination, >> 40: num_map_types >> 41: }; > > 1. Types generally seem to use PascalCase in this library, and constants > UPPER_CASE or CamelCase. > 2. You can use `enum class` to prevent the constants from polluting the > global namespace. > 3. The underlying type doesn't need to be specified, it's `int` by default. > You're casting it to `int` in `D3DMeshView.cc` anyway. About 2, I tried that initially and got into problems iterating over the enum in `D3DMeshView.cc` line 293. How would you perform the iteration? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1281#discussion_r1390020946
