Set default SVG currentColor to black

[DLTKJoelJohansson]

An attempt at fixing #3398.
Let me know what you think.

Thanks!

[DLTKJoelJohansson]

Cant get the build run locally, and jenkins reporting build errors, maybe someone can take a look? Thanks

[merks]

Don't do merge commits; we can't merge this into master in the end because we don't accept merge commits. In fact don't do multiple commits in general. Use rebase. Amend you commit and force push it when you need to make subsequent follow up changes. You'll need to rebase and force push to repair this PR.

[merks]

I'll approve the workflow so you can see any errors in the GitHub actions. Please rebase and force push to get this back on track.

[merks]

See we even fail with a check for merge commits:

[github-actions]

Test Results

  200 files  ±0    200 suites  ±0   28m 36s ⏱️ +25s
4 744 tests +1  4 720 ✅ +1   24 💤 ±0  0 ❌ ±0 
6 892 runs  +6  6 724 ✅ +6  168 💤 ±0  0 ❌ ±0 

Results for commit 74652a7. ± Comparison against base commit 26547c2.

♻️ This comment has been updated with latest results.

[merks]

I've approved the workflow again. I'll be out of office for a few hours...

[DLTKJoelJohansson]

Cant seem to figure it out, its failing for Linux builds

[DLTKJoelJohansson]

Okay got it to work now! Moved out the Display.getDefault() call to org.eclipse.swt bundle (In SVGFileFormat, the caller) fixed it.

Thanks @merks for your help. Let me know what you think

[DLTKJoelJohansson]

Maybe not, seems like the Browser tests for Windows are failing. Pretty strange

[DLTKJoelJohansson]

@merks any thoughts on this? I have a hard time seeing how these changes can cause the Browser tests to fail on Windows, since the changes only touch the SVG Rasterization

[merks]

It could just be a flaky test. I'll rerun it.

[DLTKJoelJohansson]

Looks like the same Browser tests are failing again, but at least the build succeeded now

[merks]

@HeikoKlare

I've not looked closely, but is the failure of the browser test on Windows related to this PR or maybe known to be flaky?

[joeljohansson99]

Thanks @HeikoKlare, and @merks! This PR should ready for review now

[merks]

@HeikoKlare

I will leave it to someone qualified to review the changes.

[HeikoKlare]

Thank you for proposing a solution to the mentioned issue. I have some concerns regarding the proposed solution (please see the detailed comment). What could also help is an example (e.g., a snippet or an RCP extension) that demonstrates the behavior and the change, so that we can easily test different alternatives.

[DLTKJoelJohansson]

Thanks @HeikoKlare for the feedback. I made the change so now we simply set the default foreground color to black, reverting back to the behaviour it was before the change in the underlying renderer.

I also added a regression test as you proposed. I am just checking one pixel seeing if its black. I guess you could test all pixels but I kinda felt it was overkill. Maybe there is an easier way to test this, but I could not find any.

Jenkins are failing too, not sure why. From the info I can see the compiler is saying Unnecessary @SuppressWarnings("restriction") which is very wierd since its not something I added. Any ideas what could cause this?