RFR: 8273946: Move clearQuad method to BaseShaderGraphics superclass
kcr at openjdk.java.net
Wed Sep 22 12:52:00 UTC 2021
On Tue, 21 Sep 2021 12:45:20 GMT, Kevin Rushforth <kcr at 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.
More information about the openjfx-dev