One downside to removing the interfaces is that there are parts of the code which get a type safe access to a type of Graphics (ShaderGraphics for example) and since it is type safe, we *know* that in that part of the code we are working with a graphics object that can handle shaders. We weren't accidentally given some other type of Graphics object. Since there are relatively few casts related to the graphics objects, and very few instanceof checks, maybe it is better to keep the interfaces. We can still collapse down BaseGraphics and BaseShaderGraphics, but that would be mostly it (and the BaseContext / BaseShaderContext).
Richard On Jul 23, 2013, at 10:55 AM, Richard Bair <richard.b...@oracle.com> wrote: > There are 14 types of com.sun.prism.Graphics (actually there is a 15th, > PrinterGraphics, which is a graphics by name only since it is a tagging > interface that doesn't extend from Graphics). Some of the methods in these > classes and interfaces are documented, most of them are not, which makes > understanding the code difficult (especially a couple methods in > ShaderGraphics like drawTextureRaw2). Also, the architecture here is that we > have Graphics interfaces and concrete implementations, and a lot of > instanceof checks throughout the code to differentiate one type of Graphics > object from another. > > (Which is extra interesting in that the com.sun.prism.paint package goes to > great lengths to avoid instanceof checks between different types of paints, > but here we use instanceof a lot). > > There are also some methods which look like there aren't being used at all > (such as setWindowProjViewTx, which is also undocumented). > > http://cache.fxexperience.com/wp-content/uploads/2013/07/Graphics-Hiearchy-Current.png > > I worked up the inheritance hierarchy for Graphics and this is what it looks > like. The colored round-rects are interfaces, the white square rects are > classes (either abstract or concrete). The concrete class hierarchy looks > fine to me -- having a different ES2Graphics from D3DGraphics from > TestGraphics from DummyGraphics from J2DGraphics etc all seems fine > (required, really, given our current pipeline architecture). Its the > interfaces (and what is defined on them) that I'm not finding very useful. I > think there are a couple questions worth asking: > > SWGraphics and J2DPrismGraphics (should be renamed J2DGraphics for > consistency) don't extend from BaseGraphics. In fact, only BaseShaderGraphics > extends from BaseGraphics. This implies that either BaseGraphics & > BaseShaderGraphics should be combined, or functionality that really is common > between shader based implementations and J2D and SW should exist in > BaseGraphics and the hierarchy modified accordingly. TestGraphics can just as > easily come off BaseShaderGraphics. > MaskTextureGraphics is only used by web view (1 instanceof check) > ReadbackGraphics has two instanceof checks, both in NGNode. One of these is a > method called isReadbackSupported. > RectShadowGraphics has one use in NGRectangle (1 instanceof check) > ShaderGraphics is not used in any instanceof checks, and only 1 cast. This is > a good sign this interface is not needed. > PrinterGraphics has 4 instanceofs. > > There aren't that many instanceof checks overall, which is a good thing (no > performance penalty, really). The question is whether having interfaces at > all here is buying us anything, and what exactly that might be. A couple > methods returning booleans could also be used for determining feature support > (and in fact we already do this in other places like > ReadbackGraphics#canReadBack). Reducing class count is desirable because the > more classes, the longer the startup costs and the more perm gen space used > by the VM. > > I'm thinking we should instead have a hierarchy that looks like this: > > http://cache.fxexperience.com/wp-content/uploads/2013/07/Graphics-Hierarchy-Modified.png > > Its only 6 fewer classes (all interfaces) but it flattens things out a bit > more and makes it a little more obvious who is extending what, and for what > reason. > > Richard
