-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewrite injection of CTCGraphicsEnvironment #16
Conversation
… breaks in Java 18+. Instead, use ByteBuddy to return CTCGraphicsEnvironment when sun.awt.PlatformGraphicsInfo.createGE() is called.
Avoid nonfinal
…ugh sun.awt.PlatformGraphicsInfo.createGE()
Fix unsupportedoperationexception
…by ByteBuddy interceptors. Needed to change PlatformWindowFactory from an interface into a plain class, because they are needed in the boostrap class loader (interfaces and abstract classes cannot be injected). Some other classes need to be public for the same reason.
Avoid nonfinal
restore manipulation of sun.java2d.SurfaceManagerFactory
<source>17</source> | ||
<target>17</target> | ||
<source>${cacio.java.version}</source> | ||
<target>${cacio.java.version}</target> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to define this once, I'm a bit of a Maven newbie, doesn't this need to be added to a properties section somewhere?
@@ -40,6 +40,10 @@ | |||
<module>cacio-tta</module> | |||
</modules> | |||
|
|||
<properties> | |||
<cacio.java.version>17</cacio.java.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
disregard my other comment, I see it now
<groupId>net.bytebuddy</groupId> | ||
<artifactId>byte-buddy-agent</artifactId> | ||
<version>1.14.11</version> | ||
</dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if these should have a provided
scope or perhaps shadow the jar. In my case I'll want to use whichever version the Spring Boot bom defines. I'll leave this for now but might consider that change later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am less familiar with Spring Boot and boms. I generally go for the simplest thing that works and makes sense to me.
Thank you so much for this contribution! It seems to work well and ends up being cleaner than the hack I had in for the final field. I'll get this merged and do a release in the next day or two. |
You are very welcome. In the last 5 years I came across 2 Swing projects that lacked any decent automated testing. This project is so helpful in being able to run Swing tests headlessly, so I am glad to be able to contribute. |
The former implementation (making java.awt.GraphicsEnvironment$LocalGE.INSTANCE non final and setting it to an instance of CTCGraphicsEnvironment) no longer works in JDK18+ because the JVM no longer allows removing final modifiers (UnsupportedOperationException is thrown).
The proposed implementation uses method interception through ByteBuddy to have two Swing/Awt method return an instance of CTCGraphicsEnvironment:
The latter was a bit of a challenge since it is loaded by the boostrap classloader of the JVM. This requires using ByteBuddyAgent, and also required that all needed cacio-tta classes to be loaded by the bootstrap classloader.
To make repeated testing across platforms and JVMs a bit easier the Java version used was made into a property.
Tested with JDK 17 and 21, on Linux (Ubuntu 22.04), MacOS (14.3) and Windows 10.
Solves #14
It should be noted that building with JDK 21 generates a number of deprecation warnings, making more work necessary to support JDK 22+ likely :-(