On Tue, 21 Sep 2021 12:45:20 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
> As mentioned in JBS, the `clearQuad` methods in `D3DGraphics` and > `ES2Graphics` are now identical, which should have been the case all along. > The fact that they weren't identical was the source of a bug that was only > discovered during the testing of > [JDK-8090547](https://bugs.openjdk.java.net/browse/JDK-8090547) on the es2 > pipeline. > > This PR moves the `clearQuad` method to the `BaseShaderGraphics` superclass > to get rid of the unnecessary code duplication. This will be helpful when > implementing the metal pipeline as well. As a note, I made the `context` > field in the `D3DGraphics` final, which it should have been. This is > necessary to ensure that moving the method to the super-class is equivalent. > > No new tests are needed, since this is just refactoring. Thanks for the review. > Two unrelated comments that I have observed during the review: > > 1. `java.security.AccessController` is deprecated for removal since 17. We > need to do something about this. It's used in `BaseShaderGraphics`, which is > where I saw the warning, but it's used in other places too. We added `@SuppressWarnings("removal")` annotations for [JDK-8264139](https://bugs.openjdk.java.net/browse/JDK-8264139), so I'm surprised the IDE is still flagging it. FWIW, it will be several years before we can remove any of these calls, since running with a security manager is still supported in JDK 17 LTS. > 2. Eclipse graciously informs me that "The method `D3DGraphics.getContext()` > does not override the inherited method from `BaseShaderGraphics` since it is > private to a different package". It detected the `@Override` annotation there > and noted that the superclass method is not visible to this class since they > are in different packages, so it is not actually overriding. This could lead > to another subtle bug. Maybe that method should be protected. I didn't > investigate much further. Interesting. I guess the only reason the IDE is warning in this case is that it notices the `@Override` annotation, which is there because it implements the method in D3DGraphicsContextSource, and warns that it doesn't override the method of the same name in the superclass. We could consider a follow-up issue to clean this up, but I'm not really inclined to change anything. ------------- PR: https://git.openjdk.java.net/jfx/pull/628
