RFR: 8273946: Move clearQuad method to BaseShaderGraphics superclass

Nir Lisker nlisker at openjdk.java.net
Tue Sep 21 13:50:34 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.

Looks good. The tests pass.

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.
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.

-------------

Marked as reviewed by nlisker (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/628


More information about the openjfx-dev mailing list