Skip to content
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

Fix for Mac drawing bug issue 1012 part 2 #1232

Merged
merged 1 commit into from
May 16, 2024

Conversation

Phillipus
Copy link
Contributor

- OS.objc_msgSend(id, OS.sel_setClipsToBounds_, true) needs to be applied in two more cases

- See eclipse-platform#1012
@Phillipus Phillipus requested a review from lshanmug as a code owner May 15, 2024 12:19
@Phillipus
Copy link
Contributor Author

This applies the fix in two more places in the Widget class and fixes the blank areas seen here.

However, I wonder if we are playing whack-a-mole with this and the real fix is to call OS.objc_msgSend(id, OS.sel_setClipsToBounds_, true) in one place:

public NSView(long id) {
	super(id);
	OS.objc_msgSend(id, OS.sel_setClipsToBounds_, true);
}

But as explained in #1081 (comment) we can't add it there.

@lshanmug WDYT?

Copy link
Contributor

Test Results

   418 files  ±0     418 suites  ±0   7m 42s ⏱️ +15s
 4 121 tests ±0   4 113 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 313 runs  ±0  16 221 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit 89889e3. ± Comparison against base commit d3df796.

@Phillipus
Copy link
Contributor Author

Actually, can someone merge this so we can get it in for 4.32? Without this fix, users are still going to see blank areas on Mac when using a JDK linked to Mac SDK 14.

Perhaps we can look at implementing it in NSView after 4.32?

@lshanmug
Copy link
Member

Actually, can someone merge this so we can get it in for 4.32? Without this fix, users are still going to see blank areas on Mac when using a JDK linked to Mac SDK 14.

Perhaps we can look at implementing it in NSView after 4.32?

@Phillipus Ideally the call to setClipsToBounds should be done in at the time of initialization to cover all cases. But, this change looks good for 4.32.

@lshanmug lshanmug merged commit a39c321 into eclipse-platform:master May 16, 2024
14 checks passed
@Phillipus
Copy link
Contributor Author

@Phillipus Ideally the call to setClipsToBounds should be done in at the time of initialization to cover all cases. But, this change looks good for 4.32.

@lshanmug Thanks for your feedback and the merge!

@Phillipus
Copy link
Contributor Author

Ideally the call to setClipsToBounds should be done in at the time of initialization to cover all cases. But, this change looks good for 4.32.

@lshanmug I opened #1235 to track that.

@lshanmug
Copy link
Member

Ideally the call to setClipsToBounds should be done in at the time of initialization to cover all cases. But, this change looks good for 4.32.

@lshanmug I opened #1235 to track that.

@Phillipus Thanks for the fix! I'm also thinking how to address the issue, will take a look next week and update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants