Temporary workaround for LineNumberRulerColumn regression on Linux

[akoch-yatta]

Having a debug session together with @HeikoKlare we got some insights about the underlying issue.

This PR is currently proposing having a platform-depending behavior for creating the main buffer image. Since we could identify the underlying issue in the Image implementation of GTK, we partially revert to the old implementation of the rule on Linux/GTK while using the updated one for Windows and MacOS. What this PR does in more detail:

Windows

It still uses the same logic as the initial PR. Reason: the line number ruler is not useable in the windows implementation with the monitor specific scaling enabled. As the previous implementation relied internally on the DPIUtil::getDeviceZoom we need a dynamic image creation for Windows. For that reason we really would like to have this logic in M2 for further evaluation

Mac OS

We have no issues with the changes from the PR to the ruler after eclipse-platform/eclipse.platform.swt#1751, so we stick to the state after the PR.

GTK

We are running into the issue https://bugs.eclipse.org/bugs/show_bug.cgi?id=571166 again, because we are now using another init method as before, which uses Cairo.cairo_image_surface_create that is known to produce problems in the usage context of the line number ruler. We didn't have a proper solution until now to adapt the logic (inside the Image implementation for GTK) to adress this issue, but are working on it for M3.

Question from us is. Would this workaround acceptable as temporary "fix" for M2 or would we need to revert the whole LineNumberRuler changes, accepting the still broken ruler in windows with the monitor specific scaling active

@HeikoKlare please extend on my explanation if you think something is missing

[iloveeclipse]

Question from us is. Would this workaround acceptable as temporary "fix" for M2

Yes.

[iloveeclipse]

And the patch works for me on RHEL 9.2

[github-actions]

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 31m 38s ⏱️ - 5m 25s
 7 715 tests ±0   7 487 ✅ ±0  228 💤 ±0  0 ❌ ±0 
24 306 runs  ±0  23 557 ✅ ±0  749 💤 ±0  0 ❌ ±0 

Results for commit fceb320. ± Comparison against base commit 4a38dc8.

♻️ This comment has been updated with latest results.

[iloveeclipse]

Is this ready for review now?

[HeikoKlare]

Yes, this should be ready for review (correct, @akoch-yatta?). I have updated the workaround to only be applied on Linux, as the reported MacOS bug had an independent cause that was properly fixed with eclipse-platform/eclipse.platform.swt#1751.

I have tested this workaround on Ubuntu 22.04 with X11 and found no issues with the line numbers so far. But I plan to also test with more complex monitor setups with different monitor scalings to ensure that are no regressions occur in such setups with the workaround as well.

[akoch-yatta]

Yes, we are not able to find a proper fix for the GTK issue in SWT today for M2, so this workaround would be ready to review from my side.

[iloveeclipse]

Seem to work on GTK, I've tried various use cases including git blame & compare editors (known to uncover issues in line ruler). However see my comment on removed null check.

[HeikoKlare]

I tested with different scaling settings now and also found no issues there. The only drawback is that the improvement of the line number ruler regarding scaling on GTK (reported in the original PR #2722 (comment)) is also reverted by this.

[iloveeclipse]

The only drawback is that the improvement of the line number ruler regarding scaling on GTK (reported in the original PR #2722 (comment)) is also reverted by this.

Well, that still could be contributed by someone :-)

My point is that at least basic things should always work, so that one could use every nightly build in production (surely I do not propose it for everyone but I personally use every nightly), there will be always some corner cases that need extra time but that is OK.

[HeikoKlare]

My point is that at least basic things should always work, so that one could use every nightly build in production (surely I do not propose it for everyone but I personally use every nightly), there will be always some corner cases that need extra time but that is OK.

Absolutely. No change should introduce regressions regarding existing, in particular essential functionality. So it was out of question that we either have to fully revert the change to the line number ruler or find a fix for it soon. I am still not sure why we did not experience or perceive the issue when originally testing the PR, as we did testing for all three OS. Anyway, since we only have have positive feedback on this workaround, let's merge this for M2.

The improvement of HiDPI behavior for the line number ruler on GTK with the original PR was not even intended, but rather a side effect of the improvements for Windows :-)
Since we need to find a solution for the issue arising from the usage of Cairo.cairo_image_surface_create in Image to get rid of this workaround, the HiDPI improvement will then implicitly be restored.

[iloveeclipse]

I am still not sure why we did not experience or perceive the issue when originally testing the PR, as we did testing for all three OS

Because you didn't used it for work where you need correct line numbers. That's the reason I use every nightly for daily work.

[HeikoKlare]

That's true. And that's always the difficulty / risk with changes relying on consistent cross-platform behavior, as there are places where the behavior (usually of SWT) is different across the platforms (like here). I usually work on Windows (also on the nightly version for daily work) and only use MacOS and Linux for cross-platform implementation / testing.