Added equals function to MaterialModelV2

[LocutusV0nB0rg]

The added function returns true if the object is the very same one or if all attributes are the same.

This function is very useful if one wishes to avoid multiple copies of the same material to one file. I found myself working very awkwardly with custom "contains" functions for maps and lists. Which breaks their strongly optimised usecases.

[javagl]

I considered this. And a "JglTF 3.0.0" could be a chance to include such breaking changes. (There's no fixed timeline for that yet - I still hope that I can create a new release soon...). But establishing a concept for "equality" here is not straightforward.

For now, this PR only changes to the MaterialModelV2. Maybe MaterialModelV1 is considered to be "too legacy" to touch it. But one could want to establish a similar concept of "equality" for a other model classes. (In fact, one of them already exists - but coincidentally, only for the only top-level glTF structure that does not have an equivalent ...Model class - namely, the sampler...).

For example, one could reasonably want to define such an equals function for TextureModel: They are equal when all properties are equal, including the ImageModel. And two ImageModel objects could be equal when they contain the same binary data. And one could define two AccessorModel objects to be equal, when they ... contain the same values... or maybe when they also refer to the same buffer view? 🤔

In any case, there are general questions that come up for all model classes, including the MaterialModelV2: It extends AbstractNamedModelElement meaning that it can also contain a name, extras, and extensions. Should these be ignored in such an equals implementation? Probably not. Users could explicitly assign two different names to materials, and expect them to be retained. The extras are their own beast, this can not sensibly be handled. But the extensions are crucial: When there are two materials, and one of them contains, for example, an KHR_materials_dispersion extension and the other one does not, then these materials should definitely not be considered to be 'equal'. Now, one could compare the extensions, pragmatically, and rely on their implementation of the equals function. But that's a bit of a stretch, and I would want to think this through very thoroughly.

So it's unlikely that this PR would be merged.

---

Even if all these questions could be answered, I would not merge it in its current form. In every implementation, equals MUST be consistent with hashCode: The documentation clearly says

If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result.

and given that hashCode was not overridden accordingly, this is not the case here...

[LocutusV0nB0rg]

You make some strong points which I cannot easily refute.

How about removing the "final" keyword from the class definitions? This would allow users to define their own equals functions to their liking as needed.

[javagl]

This would just make it easier for people to "break" things by overriding functions inconsiderately (but maybe that shouldn't be too much of my concern).

There are currently two things ... "in progress" (when I have time...) that will involve breaking changes:

• Update API from float to double #126 - This is does not have structural changes, but only low-level ones (meaning that the update of clients should be trivial)
• Extension support #109 - This is more connected to this PR..

The second one (for which I still haven't opened a PR with the current state) will involve some breaking changes in terms of the structures, e.g. some new functions in existing interfaces and such. And the last comment there already shows what could be the connection to the question about the equals: The model classes will have to take extensions into account in many places, and any concept of "equality" will only be one of them.

I might close this PR later and open a dedicated issue for talking about equals, but maybe I'll just leave it open as a reminder for now, just to have it on the radar for upcoming changes.

[LocutusV0nB0rg]

I might close this PR later and open a dedicated issue for talking about equals, but maybe I'll just leave it open as a reminder for now, just to have it on the radar for upcoming changes.

Feel free to do as you like, I'll keep an eye open for when it is release. If you could mention the implementation in the release note of such a version, I'd very much appreciate it.

This would just make it easier for people to "break" things by overriding functions inconsiderately (but maybe that shouldn't be too much of my concern).

To be honest, I could implement another implementation of the MaterialModel interface , but I was just too lazy. This would break stuff too. Of course the decision is yours, but things are already "breakable" now.

Extensions are a whole other beast that I didn't even consider, when creating this MR in the first place. I don't have enough experience to suggest a solution for that, at least for now an extensible class seems the least breaking to me.

[javagl]

I could implement another implementation of the MaterialModel interface , but I was just too lazy. This would break stuff too.

The transition between glTF 1.0 and 2.0, combined with some naivety and idealism on my side, led to the current state: All structures are defined via interfaces. Ideally, no internal code should rely on a particular implementation of the interface. But some of the code does rely on that (the Default... ones, or MaterialModelV2). Sooo... yeah, maybe I should just shrug it off. But I'll try to think about a few points here in the context of the extension handling, maybe there are alternatives to that.