+1

Constants good enough for now. First pass doesn't really need subclasses since the 4 constants we start with are virtual and somewhat opaque, but we could put them in for grins if we want.

But, when we start making the different AA types selectable and parameterized then I think we want to have subclasses at that point. The concepts of "where are the subsamples taken" (both in respect to the positioning of the N sub-samples that determine coverage and also which of them, if any, are used to execute the shader) in MSAA have no bearing on FSAA, for example. Trying to mix and match disparate descriptive parameters in a single class gets awkward...

                        ...jim

On 7/31/13 5:09 PM, Richard Bair wrote:
Sorry let me be clear. Having a class vs an enum was my preferred approach and 
I'm glad with Jim's nudge it looks like we'll go there. Having just the few 
predefined constants as a starting point I think is good, and we can add 
sub-types and more goodness in the future.

Richard

On Jul 31, 2013, at 4:47 PM, Richard Bair <richard.b...@oracle.com> wrote:

I'm pretty confident we'll want different sub-types as we go along (CSAA, MSAA, 
FXAA -- there are a lot of different ways to do full-scene anti-aliasing and I 
bet that they will have different parameters for controlling their various 
algorithms), but we can cross that bridge later.

Richard

On Jul 31, 2013, at 4:36 PM, Chien Yang <chien.y...@oracle.com> wrote:

I agree, however I would prefer a single class over subclasses if possible. I 
have added Jim's proposal to the JIRA for consideration.

https://javafx-jira.kenai.com/browse/RT-31878

Thanks,
- Chien



On 7/31/2013 3:21 PM, Kevin Rushforth wrote:
This seems cleaner in terms of extensibility. I think we can wait on adding 
anything other than the public static finals for this release, but plan to 
extend it using something like what Jim suggests.

-- Kevin


Richard Bair wrote:
Personally I liked this approach. It was like an enum in ease of use but much 
more extensible in the future when we add more anti-aliasing types and twiddles.

Richard

On Jul 31, 2013, at 1:36 PM, Jim Graham <james.gra...@oracle.com> wrote:

D'oh!  I knew I should have been checking this list a bit.  I hope this isn't 
too late to have any impact...

As an intermediate solution this is fine, but when we want to get into 
providing settings for MSAA and FSAA and other algorithms I think classes are 
more flexible than enums.  How about this solution:

package javafx.?

public class SceneAntialiasing {
  public static final SceneAntialiasing DISABLED;
  public static final SceneAntialiasing BALANCED;
  public static final SceneAntialiasing FASTEST;
  public static final SceneAntialiasing NICEST;

  public static SceneAntialiasing[] getAvailableTechniques() { }

  SceneAntialiasing() { /* package private constructor! */ }
}

public class MsaaAntialiasing extends SceneAntialiasing {
  MSaaAntialiasing(int numsamp) { /* package private */ }
  public int getNumSamples();
}

public class FsaaAntialiasing extends SceneAntialiasing {
  FsaaAntialiasing(int numsamp) { /* package private */ }
  public int getNumSamples();
}

Note that there are ways for the system to construct these objects without 
providing public constructors so that these become statically defined by the 
system.

What about Anisotropic filtering?  Is that considered a form of AA, or an 
option on top of AA?

           ...jim

On 7/24/2013 3:07 PM, Chien Yang wrote:
Thanks for the help! I was of 2 minds about it; alphabetical or logical.

public enum SceneAntiAliasing {
  DISABLED, // disables anti-aliasing
  BALANCED, // enables anti-aliasing using optimal system setting available 
that balances speed and quality
  FASTEST, // enables anti-aliasing using minimum system setting available that 
results in better frame rate
  NICEST // enables anti-aliasing using maximum system setting available that 
results in best visual quality
}

- Chien

On 7/24/2013 2:49 PM, Richard Bair wrote:
Just to be picky, I would put DISABLED first in the list. It seems more consistent 
to have the only OFF mode to be first and then all the rest of the options (which 
happen to then have ordinals > 0) will be some form of ON mode.

Richard

On Jul 24, 2013, at 2:37 PM, Chien Yang <chien.y...@oracle.com> wrote:

Thank you for the feedback! We decided to drop DEFAULT in favor of BALANCED. So 
here is the revised SceneAntiAliasing enum entries:

public enum SceneAntiAliasing {
  BALANCED, // enables anti-aliasing using optimal system setting available 
that balances speed and quality
  DISABLED, // disables anti-aliasing
  FASTEST, // enables anti-aliasing using minimum system setting available that 
results in better frame rate
  NICEST // enables anti-aliasing using maximum system setting available that 
results in best visual quality
}

Thanks,
- Chien

On 7/23/2013 1:29 PM, Chien Yang wrote:
Hi all,

  We appreciate all the feedback you have contributed to this topic. After 
listening to the feedback and an internal discussion, we would like to propose 
a minor change to the API for supporting scene anti-aliasing. We intentionally 
choose not to expose the number of samples and techniques used in this release, 
but this doesn't preclude future addition when the time is right for more 
options. This change will be tracked by RT-31878 
(https://javafx-jira.kenai.com/browse/RT-31878):

Anti-aliasing API Change Proposal:

Constructors remove:
public Scene(Parent root, double width, double height, boolean depthBuffer, 
boolean antiAliasing)
public SubScene(Parent root, double width, double height, boolean depthBuffer, 
boolean antiAliasing)

Constructor add:
public Scene(Parent root, double width, double height, boolean depthBuffer, 
SceneAntiAliasing antiAliasing)
public SubScene(Parent root, double width, double height, boolean depthBuffer, 
SceneAntiAliasing antiAliasing)

Note:The antiAliasing argument will be used if the underlying graphics driver 
has anti-aliasing support.

Where SceneAntiAliasing is an enum with the following entries at the moment:

public enum SceneAntiAliasing {
  DISABLED, // disables anti-aliasing
  DEFAULT, // enables anti-aliasing using a default system setting available 
that balances speed and quality
  FASTEST, // enables anti-aliasing using minimum system setting available that 
results in better frame rate
  NICEST // enables anti-aliasing using maximum system setting available that 
results in best visual quality
}

Thanks,
- Chien




Reply via email to