introduce static helper method GC#drawOn #1063

tobiasmelcher · 2024-02-26T07:44:12Z

which takes care of creation and disposal of the GC object

Fixes: #955

tobiasmelcher · 2024-02-26T08:05:11Z

@HannesWell : could you please take a look at this change? I had to introduce a new Runnable interface which passes the GC as argument. Is there already an existing interface I could re-use? Do we have to introduce the static method GC#drawOn in all platform specific GC implementations?

github-actions · 2024-02-26T08:22:23Z

Test Results

299 files ±0 299 suites ±0 6m 58s ⏱️ -21s
4 100 tests ±0 4 092 ✅ ±0 8 💤 ±0 0 ❌ ±0
12 212 runs ±0 12 137 ✅ ±0 75 💤 ±0 0 ❌ ±0

Results for commit 573159d. ± Comparison against base commit e703afe.

♻️ This comment has been updated with latest results.

HannesWell · 2024-03-23T10:49:05Z

Hello @tobiasmelcher, thank you for this PR. Sorry for my late reply but I just started my almost three week of vacation when you created this PR and when I came back a full TODO list was ready to be worked off.

Do we have to introduce the static method GC#drawOn in all platform specific GC implementations?

Yes we do, but you can just import/open the fragments for the other platforms and they will compile in your workspace. For you it is probably sufficient to only have one arch per OS since they only differ in the embedded native binaries, but the code is the same for all archs on one OS.

I had to introduce a new Runnable interface which passes the GC as argument. Is there already an existing interface I could re-use?

Yes there is already SWTRunnable. But as I have just written in #955 (comment), I think this is not the optimal solution, which I only noticed when reviewing your changes to apply the new method (applying it immediately can therefore be very helpful 👍🏽 )

HannesWell

Thanks for the update. In general it looks good, but I added some remarks below.

But the most important missing part is the implementation for the linux- and mac-platform as written in my previous comment.

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java

...g.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/AnimatedProgress.java

...les/org.eclipse.swt/Eclipse SWT Custom Widgets/common/org/eclipse/swt/custom/CTabFolder.java

tobiasmelcher · 2024-03-28T22:10:25Z

Thanks a lot @HannesWell for all the feedback. Could you please take a look and review my latest change eb1dc57 ? As already mentioned, I had some troubles with the formatting; I executed then the format action only on the changed coding parts. Is that ok?

HannesWell

As already mentioned, I had some troubles with the formatting; I executed then the format action only on the changed coding parts. Is that ok?

Yes, that's IMO the perfect decision.

Could you please also update the doc of the GC class to mention the new GC.create() factory (for all platforms).

With that I think this change is complete (I'll mark it as ready for review), but I would like give others some time to review.

HannesWell · 2024-03-29T13:02:14Z

bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/graphics/GC.java

@@ -59,7 +59,22 @@
 * @see <a href="http://www.eclipse.org/swt/examples.php">SWT Examples: GraphicsExample, PaintExample</a>
 * @see <a href="http://www.eclipse.org/swt/">Sample code and further information</a>
 */
-public final class GC extends Resource {
+public sealed class GC extends Resource {


It would be good to also update the doc of this class (where it mentions GC.dispose()) and recommend to use the new factory, ideally with the little example also used in the Image class.
And I think Windows95 and Windows98 are not the most relevant target OS anymore. Maybe that just should be generalized.

HannesWell · 2024-03-29T13:05:06Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java

+ * try (var gc = GC.create(i)) {
+ *  gc.drawRectangle(0, 0, 50, 50));
+ * }


Please apply this doc update also to the Image implementations for Linux and Mac.

which takes care of creation and disposal of the GC object Fixes: eclipse-platform#955

- format the changed coding parts - remove the GC.create calls at locations where the diff gets too large

HannesWell · 2024-03-30T11:28:11Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java

+ }
+ }
+
+ public static GC.Closeable create(Drawable drawable) {


The new method needs some java-doc on all platforms. The existing single argument constructor of GC is probably a good source of inspiration.
The existing GC constructor could also be adjusted to recommend this method for safety.

What's important is to add the tag @since 3.126 in the Java-doc in order to make the API-tools aware this method was just added.

Thanks for the hint. I hope it is fine with 6443eca

HannesWell · 2024-03-30T11:29:45Z

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java

-public final class GC extends Resource {
+public sealed class GC extends Resource {
+
+ public static final class Closeable extends GC implements AutoCloseable {


This new class needs some java-doc on all platforms. I think a short text that mentions that this is an auto-closable extension of the GC class is sufficient. The create method could be linked with @see.
What's important is to add the tag @since 3.126 in the Java-doc in order to make the API-tools aware this method was just added.

tobiasmelcher requested review from niraj-modi and iloveeclipse as code owners February 26, 2024 07:44

tobiasmelcher marked this pull request as draft February 26, 2024 07:47

BeckerWdf requested a review from HannesWell February 26, 2024 08:03

BeckerWdf force-pushed the GC.drawOn branch from b27a84d to d4fb76f Compare February 26, 2024 08:16

HannesWell mentioned this pull request Mar 23, 2024

Implement java.lang.AutoCloseable in class org.eclipse.swt.graphics.GC #955

Open

tobiasmelcher force-pushed the GC.drawOn branch from d4fb76f to 37519e2 Compare March 28, 2024 17:31

HannesWell requested changes Mar 28, 2024

View reviewed changes

HannesWell reviewed Mar 29, 2024

View reviewed changes

HannesWell marked this pull request as ready for review March 29, 2024 13:38

HannesWell requested review from lshanmug and sravanlakkimsetti as code owners March 29, 2024 13:38

tobias-melcher added 3 commits March 29, 2024 18:45

introduce static helper method GC#drawOn

fd3f755

which takes care of creation and disposal of the GC object Fixes: eclipse-platform#955

introduce GC.create which returns an AutoCloseable

e3a6ef5

introduce GC.Closeable also in cocoa and gtk

573159d

- format the changed coding parts - remove the GC.create calls at locations where the diff gets too large

HannesWell force-pushed the GC.drawOn branch from eb1dc57 to 573159d Compare March 29, 2024 17:46

HannesWell reviewed Mar 30, 2024

View reviewed changes

add java doc

6443eca

tobiasmelcher force-pushed the GC.drawOn branch 2 times, most recently from eb1dc57 to 6443eca Compare April 2, 2024 07:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce static helper method GC#drawOn #1063

introduce static helper method GC#drawOn #1063

tobiasmelcher commented Feb 26, 2024

tobiasmelcher commented Feb 26, 2024

github-actions bot commented Feb 26, 2024 •

edited

HannesWell commented Mar 23, 2024

HannesWell left a comment

tobiasmelcher commented Mar 28, 2024

HannesWell left a comment

HannesWell Mar 29, 2024

HannesWell Mar 29, 2024

HannesWell Mar 30, 2024

tobiasmelcher Apr 2, 2024 •

edited

HannesWell Mar 30, 2024

introduce static helper method GC#drawOn #1063

Are you sure you want to change the base?

introduce static helper method GC#drawOn #1063

Conversation

tobiasmelcher commented Feb 26, 2024

tobiasmelcher commented Feb 26, 2024

github-actions bot commented Feb 26, 2024 • edited

Test Results

HannesWell commented Mar 23, 2024

HannesWell left a comment

Choose a reason for hiding this comment

tobiasmelcher commented Mar 28, 2024

HannesWell left a comment

Choose a reason for hiding this comment

HannesWell Mar 29, 2024

Choose a reason for hiding this comment

HannesWell Mar 29, 2024

Choose a reason for hiding this comment

HannesWell Mar 30, 2024

Choose a reason for hiding this comment

tobiasmelcher Apr 2, 2024 • edited

Choose a reason for hiding this comment

HannesWell Mar 30, 2024

Choose a reason for hiding this comment

github-actions bot commented Feb 26, 2024 •

edited

tobiasmelcher Apr 2, 2024 •

edited