[GTK] Initialize text-antialias when initializing GC object

[ptziegler]

On some platforms, the default text-antialias of the system might differ from Cairo. When creating a new GC object is created, the Cairo default is used. But when setTextAntialias(SWT.DEFAULT) is called, the system default is used instead.

To avoid this ambiguity, setTextAntialias(SWT.DEFAULT) should always be called when creating a GC object, in order to always use the system default.

[ptziegler]

@HeikoKlare This is the problem I mentioned in #1757. I genuinely believe that this is an issue in Cairo, given that it only happens on some platforms.

The only workarounds I found to fix this change is by either reverting your changes to the Image class or by explicitly calling setTextAntialias(SWT.DEFAULT) before drawing the first string.

[github-actions]

Test Results

   506 files  ±0     506 suites  ±0   8m 56s ⏱️ +44s
 4 340 tests +1   4 326 ✅ +1   14 💤 ±0  0 ❌ ±0 
16 599 runs  +4  16 490 ✅ +4  109 💤 ±0  0 ❌ ±0 

Results for commit 949d963. ± Comparison against base commit 0368a6b.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Sorry, I saw this one after writing a comment to the other PR. Please see #1835 (comment)
I don't think this is the right thing to do. We should rather focus on finding the actual issue and fix that. This proposed change might only serve as a temporary rollback for the upcoming release.

[ptziegler]

@HeikoKlare I investigated this problem further on my Ubuntu VM and I believe the cleanest solution is to explicitly call setTextAntialias() when creating a GC object, in order to make sure that Cairo is using the system text-antialias, rather than its own default setting.

[HeikoKlare]

Sorry for the late response. From what you explained it might be necessary to explicitly set text antialiasing as proposed, but that should probably not happen in GC directly (so that it affects every GC) but rather in the GC creation specific for image (i.e., in Image#internal_new_GC), shouldn't it?

Anyway, I think there is quite some risk in such a change, so we should properly test it throughout a release cycle. Thus, I would be in favor of merging your orignal change (reverting the change to the Image class), as that should be safe to do, and then reapply that change together with a fix for the text antialiasing once development opens for 2025-06. What do you think?

[ptziegler]

but rather in the GC creation specific for image (i.e., in Image#internal_new_GC), shouldn't it?

Sadly, that doesn't really work. The property needs to be overwritten in the Cairo context that is created by the GC, but this has then nothing to do with the image.

But you are correct that this problem doesn't exist for e.g. Shells and Composites. In that case, the value doesn't need explicitly set.

The options I see are to either always create a pango context (effectively duplicating createLayout()) when creating the Image GC or to somehow encode this information in the GCData instance.

Anyway, I think there is quite some risk in such a change, so we should properly test it throughout a release cycle. Thus, I would be in favor of merging your orignal change

I agree that the current change is too risky. But I also don't really like to only partially revert the previous commit. Given how close we are to the RC1, I'd rather leave the ruler fix in a tested state and then handle this case for the 2025-06. At least for GEF, we temporarily disabled the problematic test case, so there is no longer any real pressure from our side.

[HeikoKlare]

I agree that the current change is too risky. But I also don't really like to only partially revert the previous commit. Given how close we are to the RC1, I'd rather leave the ruler fix in a tested state and then handle this case for the 2025-06. At least for GEF, we temporarily disabled the problematic test case, so there is no longer any real pressure from our side.

Okay, sounds good. I was wondering whether we should at least merge #1835 then, as the switch to 32 bit was actually wrong, but maybe that's also too risky now that we are after RC1.

[ptziegler]

Okay, sounds good. I was wondering whether we should at least merge #1835 then, as the switch to 32 bit was actually wrong, but maybe that's also too risky now that we are after RC1.

I think my initial guess might've been a red herring. I at least could not detect any issues that could be traced back to using a 32bit, as opposed to 24bit.

For GTK3, I also would've expected the previously used gdk_window_create_similar_surface method to create a surface with 32bit depth, assuming that this is supported by the OS. So it might've been the correct decision after all.

[HeikoKlare]

For GTK3, I also would've expected the previously used gdk_window_create_similar_surface method to create a surface with 32bit depth, assuming that this is supported by the OS. So it might've been the correct decision after all.

If I am not mistaken, the previous implementation used 24bit as Cairo.CAIRO_CONTENT_COLOR requests a surface with 8 bit colors and without an alpha channel (otherwise it would be Cairo.CAIRO_CONTENT_COLOR_ALPHA):

surface = GDK.gdk_window_create_similar_surface(GDK.gdk_get_default_root_window(), Cairo.CAIRO_CONTENT_COLOR, width, height);

[ptziegler]

If I am not mistaken, the previous implementation used 24bit as Cairo.CAIRO_CONTENT_COLOR requests a surface with 8 bit colors and without an alpha channel (otherwise it would be Cairo.CAIRO_CONTENT_COLOR_ALPHA):

No, you are correct. SWT used a 24bit surface in both the GTK3 and GTK4 case. Then it makes sense to merge the other commit, even if its just to avoid any unpleasant edge cases.

[iloveeclipse]

Could you please bump SWT tests version?

[ptziegler]

Could you please bump SWT tests version?

The fragment was already updated with ccc69c2, so I don't think this is necessary anymore?

But related to the original problem. I tried to set the font options directly for the image handle in the internal_new_GC() method but so far, everything was simply ignored. After a while I found this Cairo bug, describing a similar problem, so I'm not sure whether I'm doing something wrong or whether this is something that needs to be fixed at the source.

In any case, the issue only seems to happen for images, so I've restricted the setTextAntialias() call to image-GCs.

[merks]

I believe the verification builds generally add version increment commits when they are needed. E.g., the second commit in this PR was automatic:

eclipse-platform/eclipse.platform.ui#2827

[HeikoKlare]

I believe the verification builds generally add version increment commits when they are needed.

It did not work for tests/examples so far, But we already had to do the version bump for the tests bundle for another change today, which is why it is not necessary here anymore, as mentioned by Patrick.

[merks]

Maybe it's not enabled for SWT's repo in general?

[akurtakov]

It is not enabled for equinox and swt IIRC but unfortunately I don't remember what the problem was.

[HeikoKlare]

It is not enabled for equinox and swt IIRC but unfortunately I don't remember what the problem was.

fwiw: the according PR for SWT is still open #1491