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

[GTK][HiDpi] Code cleanup for removal of non-cairo scale path #1304

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deepika-u
Copy link
Contributor

Fixes #1300

Copy link
Contributor

github-actions bot commented Jun 26, 2024

Test Results

  281 files   -   204    281 suites   - 204   5m 19s ⏱️ - 3m 32s
4 139 tests  -    19  4 123 ✅  -    26   7 💤  -  1  3 ❌ +2  6 🔥 +6 
8 209 runs   - 8 177  8 144 ✅  - 8 149  56 💤  - 36  3 ❌ +2  6 🔥 +6 

For more details on these failures and errors, see this check.

Results for commit ff050b1. ± Comparison against base commit 02e20d6.

This pull request removes 19 tests.
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letters
…

♻️ This comment has been updated with latest results.

@deepika-u deepika-u force-pushed the 1300_Code_cleanup branch 4 times, most recently from b48f0d1 to 52ca34f Compare July 10, 2024 13:36
@deepika-u deepika-u marked this pull request as ready for review July 10, 2024 13:55
@deepika-u
Copy link
Contributor Author

@SyntevoAlex @akurtakov @merks @sravanlakkimsetti
Can you review this please.

@SyntevoAlex
Copy link
Member

Sorry, I don't think I can find time to review.

@deepika-u
Copy link
Contributor Author

@akurtakov @iloveeclipse
These are the changes done as per @sravanlakkimsetti's suggestions, so can one of you review the changes please.

@deepika-u
Copy link
Contributor Author

@akurtakov @iloveeclipse : Can you review this pr please?

@akurtakov
Copy link
Member

@deepika-u It's not even compiling as can be seen in the logs:

17:31:20  [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:4.0.8:compile (default-compile) on project org.eclipse.swt.gtk.linux.x86_64: Compilation failure: Compilation failure: 

17:31:20  [ERROR] /home/jenkins/agent/workspace/eclipse.platform.swt_PR-1304/eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java:[673] 

17:31:20  [ERROR] 	height += getSpacingInPixels();

17:31:20  [ERROR] 	          ^^^^^^^^^^^^^^^^^^

17:31:20  [ERROR] The method getSpacingInPixels() is undefined for the type TextLayout

17:31:20  [ERROR] /home/jenkins/agent/workspace/eclipse.platform.swt_PR-1304/eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java:[704] 

17:31:20  [ERROR] 	int yExtent = extent ? getSpacingInPixels() : 0;

17:31:20  [ERROR] 	                       ^^^^^^^^^^^^^^^^^^

17:31:20  [ERROR] The method getSpacingInPixels() is undefined for the type TextLayout

17:31:20  [ERROR] 4 problems (2 errors, 2 warnings)

@deepika-u deepika-u force-pushed the 1300_Code_cleanup branch 2 times, most recently from bb37696 to 7aa653d Compare September 19, 2024 13:27
@deepika-u
Copy link
Contributor Author

deepika-u commented Sep 19, 2024

@deepika-u It's not even compiling as can be seen in the logs:

17:31:20  [ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:4.0.8:compile (default-compile) on project org.eclipse.swt.gtk.linux.x86_64: Compilation failure: Compilation failure: 

17:31:20  [ERROR] /home/jenkins/agent/workspace/eclipse.platform.swt_PR-1304/eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java:[673] 

17:31:20  [ERROR] 	height += getSpacingInPixels();

17:31:20  [ERROR] 	          ^^^^^^^^^^^^^^^^^^

17:31:20  [ERROR] The method getSpacingInPixels() is undefined for the type TextLayout

17:31:20  [ERROR] /home/jenkins/agent/workspace/eclipse.platform.swt_PR-1304/eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/TextLayout.java:[704] 

17:31:20  [ERROR] 	int yExtent = extent ? getSpacingInPixels() : 0;

17:31:20  [ERROR] 	                       ^^^^^^^^^^^^^^^^^^

17:31:20  [ERROR] The method getSpacingInPixels() is undefined for the type TextLayout

17:31:20  [ERROR] 4 problems (2 errors, 2 warnings)

Thanks alot for reviewing this pr. I did the needed changes to TextLayout.java and those 2 errors are gone. Sorry it was a miss from my side.

But now seeing error "The method org.eclipse.swt.graphics.Image.getBoundsInPixels() has been removed". For this if i change the version from 3.128.0.qualifier to 4.0.0, this error is gone. Should i be doing this change also? Do you also see this error with respect to Image.java and MANIFEST.MF files?

@akurtakov
Copy link
Member

@deepika-u You can not delete API. Even if it's deprecated it is not marked forRemoval and it has to stay 2 years after there is release with it marked that way. Bumping major version for that is also a no-go.

@deepika-u
Copy link
Contributor Author

@deepika-u You can not delete API. Even if it's deprecated it is not marked forRemoval and it has to stay 2 years after there is release with it marked that way. Bumping major version for that is also a no-go.

@akurtakov: Reverted the changes of Image.java, now when pr is applied i am not finding any errors. Can you please review now? Thanks alot in advance.

@akurtakov
Copy link
Member

akurtakov commented Sep 20, 2024

@deepika-u You can not delete API. Even if it's deprecated it is not marked forRemoval and it has to stay 2 years after there is release with it marked that way. Bumping major version for that is also a no-go.

@akurtakov: Reverted the changes of Image.java, now when pr is applied i am not finding any errors. Can you please review now? Thanks alot in advance.

Before asking me for review again please look at the build output . Automated build and tests are there so contributors see when things don't compile (previous case) or when tests fail(https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1304/18/) like now. Please make sure that you have checked all fails and have a clean build before asking explicitly.

@deepika-u
Copy link
Contributor Author

Please make sure that you have checked all fails and have a clean build before asking explicitly.

Thanks for letting me know on another check(which i didnt knew), i shall check on them.

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.

[GTK][HiDpi] Code cleanup for removal of non-cairo scale path
3 participants