On Fri, 10 Nov 2023 15:24:13 GMT, Nir Lisker <nlis...@openjdk.org> wrote:

>> modules/javafx.graphics/src/main/java/com/sun/javafx/scene/paint/TextureData.java
>>  line 12:
>> 
>>> 10:  */
>>> 11: // Here we can support mipmaps, wrapping modes (which exists internally 
>>> and can be pulled out), addressing modes etc.
>>> 12: public class TextureData {
>> 
>> 1. This type could be a record.
>> 2. There's no actual texture data (i.e. pixels) here, maybe a better name 
>> would be `SamplerParameters` or something like that.
>
> 1. Not when it will be promoted to public API. Adding record components 
> breaks backwards compatibility, so making this a record will not allow adding 
> more configuration later on. What might be possible is making it an interface 
> and using a record as an implementation.
> 2. I think that this class can go beyond sampler configurations. For example, 
> wrapping is a render state, not a sampler state. Maybe something more general 
> like "TextureParameters" can work.

I agree with 2. and I also think that `TextureParameters` would be a better 
name for this.

This should also follow in below method names and such (ex. 
`setTextureParameters()` instead of `setTextureData()`)

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1281#discussion_r1394084664

Reply via email to