MSAA and Scene anti aliasing

[Scott Palmer]

I would vote for option b)
Use a full class that can be extended later as needed.  You don't have to use it like an enum. It could be a container for configuration parameters, possibly including the enum that says the antialiasing mode, but with separate fields (or maybe even just a single map) to supply the additional details e.g. x2, x4, x16.
I also find that mixing the kind of antialiasing with the parameters in a single enum is smellier than keeping those things distinct, but I don't like methods with a lot of parameters either.  You could supply simple static fields or methods to get a class configured for the common cases so it could be used as easily as an enum.

Scott

On 2013-07-15, at 4:45 PM, Richard Bair <richard.bair at oracle.com> wrote:

> 
>> I think there should be a simple way to request full scene anti-aliasing to improve 3D rendering. And also an optional more advanced way to specify which type…
> 
> 
> I agree it should be nice and simple. It should also allow us the freedom to make it better in the future. I think that adding a secondary property to refine what is meant by the first property is not a good idea for the following reason. As our API stands today it is not possible to later add any API without cluttering things up. Specifically this is because we require you to specify anti-aliasing at the time the scene is constructed. Ostensibly this is because it is easier for our implementation. I think this is wrong (and the fact that we expose a read only property for anti-aliasing is going to come back to bite us if we ever attempt to make it a read-write property). But suppose we decide that no, it really should be read-only for all time. In that case, in 8.1 we decide to add more API that lets you specify various details about the implementation, such as the algorithm to use. Now we have to add yet another constructor with yet more parameters to be able to specify this additional constraint.
> 
> This is bad.
> 
> What we can do is either of the following:
>    a) Make anti-aliasing a full read-write property and get it out of the constructor
>    b) Change it to an enum (or class) so we can grow it over time
> 
> Making it a read-write property (a) would at least allow us to add another read-write property in the future (which is inevitable) without adding more and more constructors. Once upon a time the root node of a Scene was immutable and thus we had to have a non-empty scene constructor. That is no longer the case and we should be striving to make Scene something that could be completely configured dynamically (after all, we should be able to detach and chuck the peer and create a new one on demand -- if we can't, that's a failing in our implementation, not in the API). As such, it really is a bad idea for us to be adding new read-only properties here.
> 
> Separate from that discussion is the one around whether it should be a boolean or an enum. I think a boolean is short-sighted because it is inevitable that we are going to need to expose additional choices to developers. So that means we will, at a minimum, need two properties instead of one to configure the antialiasing strategy. It isn't the end of the world. Using a full class to define the AA approach is also possible (if null, no AA, otherwise it is an implementation of SceneAntiAliasing which could be MSAA or whatever, package private constructor, fixed set of types, various properties can grow on those different types over time, etc). Using a full class probably provides the most flexibility but is more cumbersome. Using an enum is just as easy programatically as a boolean, offers the opportunity to grow it over time, but isn't as flexible as using a class.
> 
> Once we ship this API we have to live with it forever, and I have a very strong feeling that we are going to be modifying this in the future, because as it stands it isn't that useful (only in the short term).
> 
> I think making this a read-write property now is a must-do, because you can't change it later (at least, you can't change the property method to return a full property, it has to remain a read-only property forever for binary compatibility reasons).
> 
>> For the advanced variation, I think it might be premature. Considering there is an ugly workaround to specify number of samples for MSAA, with “prism.multisample” system property.
> 
> We don't have to do anything prematurely. An enum with two options is perfectly acceptable in the first go-around. The question is, how are we going to handle growing this API in the future. Two properties in this case is uglier than one.
> 
> Richard