Fix for Mac drawing bug

[Phillipus]

• Add a new selector setClipsToBounds
• Set this selector to true in Widget#drawRect
• See MacOS 14.3 M2: RCP application blank screen with Temurin 17.0.10 #1012

[github-actions]

Test Results

   299 files  ±0     299 suites  ±0   7m 53s ⏱️ + 1m 24s
 4 099 tests ±0   4 091 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 209 runs  ±0  12 134 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit c99224b. ± Comparison against base commit cca2034.

♻️ This comment has been updated with latest results.

[Phillipus]

Pre-requisites to test this PR:

• macOS 14 (ARM or Intel)
• Target Java of Temurin 17.0.10+7 or later, or 21.0.2+13 or later
• Launch child Eclipse or one of the Snippets in MacOS 14.3 M2: RCP application blank screen with Temurin 17.0.10 #1012 with target Java above

[Phillipus]

Added a commit that calls setClipsToBounds in the NSView constructors rather than in Widget#drawRect

[lshanmug]

We don't add any code directly to NSView.java (and all NS*.java files) as they are auto-generated by the MacGenerator tool. Every time we run the MacGenerator, it will generate the files again and our changes will be lost.
NSView(long id) and NSView(id id) are used to create NSView object for an existing view. NSView() is called for new view creation. May be initializing in NSView() instead of NSView(long id) and NSView(id id) would work? However we can't make changes to NSView class directly.

[Phillipus]

We don't add any code directly to NSView.java (and all NS*.java files) as they are auto-generated by the MacGenerator tool. Every time we run the MacGenerator, it will generate the files again and our changes will be lost. NSView(long id) and NSView(id id) are used to create NSView object for an existing view. NSView() is called for new view creation. May be initializing in NSView() instead of NSView(long id) and NSView(id id) would work? However we can't make changes to NSView class directly.

I found that the fix only works in the NSView(long id) constructor as it's really only needed in Widget#drawRect. Initialising in NSView() won't work because id is not set at that point.

If NSView is auto-generated then I guess the setClipsToBounds method there is also not valid?

I don't really know what is the best approach here.

[lshanmug]

If NSView is auto-generated then I guess the setClipsToBounds method there is also not valid?

Yes, setClipstoBounds can't be added directly to NSView.java, it has to ideally be added through the MacGenerator tool and generated. But since it's a new symbol, it can't be added through the tool as well without updating the bridgesupport files. (for more info pls see)
But, we can directly set the property by calling OS.objc_msgSend(this.id, OS.sel_setClipsToBounds_, true);. We do similar calls already for some of the new symbols, for example, see the 10.12, 10.14, 11.0 selectors in OS.java. We can also have the wrapper setClipstoBounds in OS.java if you prefer to call the function instead of objc_msgSend to selector.

I don't really know what is the best approach here.

Let's go with the previous approach of setting the property in drawRect for now, until we find a way to initialize the property.

[Phillipus]

Pushed new commit that doesn't add anything to NSView but calls OS.objc_msgSend(this.id, OS.sel_setClipsToBounds_, true); directly from Widget#drawRect

[lshanmug]

Changes look good to me, but I've not tested it myself.

Since there are no native changes, code should compile fine with older SDK as well. The release notes mention that symbol is available till 10.9, hence no version check is required to guard the call for older SDK versions. @Phillipus please correct me if I missed something.

[Phillipus]

Changes look good to me, but I've not tested it myself.

Since there are no native changes, code should compile fine with older SDK as well. The release notes mention that symbol is available till 10.9, hence no version check is required to guard the call for older SDK versions. @Phillipus please correct me if I missed something.

@lshanmug I think your summary is correct. Thanks for your feedback!

[HeikoKlare]

The change looks good to me. With all the information given by @lshanmug, it seems to be sound to change the clipping property within the drawRect() method. It won't affect NSView in the same way as setting the property within NSView's constructor (as it is instantiated at far more places than in the drawRect() method), but since the value is only relevant on drawing, it seems to be sufficient to set it there. And doing it only where necessary also reduces the number of OS calls.

From a user's point of view, I have tested the change with an SDK product started from within an Eclipse IDE and everything worked for me as expected, no issues so far. Using the same run configuration without this fix shows the addressed issue.
MacOS: Sonoma 14.1.1
Chip: Apple M2 Pro
JDK: Temurin 17.0.10

In case there are no further concerns, I propose to merge this soon to find potential regressions or further issues as soon as possible.

[Phillipus]

Thanks for the feedback @HeikoKlare

I've also tested it on non-affected macOS (13) and non affected Java and it's OK. On macOS 13 and earlier the selector call is basically a NOP, so does nothing.

As of today, this problem will mainly affect Eclipse developers who are on macOS 14 and who target the latest versions of Temurin Java. As explained elsewhere, this is because the Temurin java binaries are linked (compiled?) with the macOS 14 SDK.

But it also affects SWT-based apps such as this one.

As of today, it won't affect Eclipse on Mac itself because the Eclipse binary launcher is linked to macOS 13. However, that probably won't be the case in the future if the build machine updates to the latest OS and SDKs. I asked the question here, but got no response.

So it would be good to get this fix in for the next release of Eclipse, and it will also benefit apps that use SWT directly.

[Phillipus]

In case there are no further concerns, I propose to merge this soon to find potential regressions or further issues as soon as possible.

Could someone merge this so we can get feedback?

[merks]

I rebased it to ensure that we aren't missing a version increment or something like that. I'll merge when it's done. Feel free to ping with a friendly reminding if I don't do this after the build completed. 😀

[Phillipus]

@merks Thanks for the rebase and merge, I appreciate it. :-)

[merks]

What goes around, comes around. 👼