Open Nancy-Salpepi opened 1 year ago
I don't have macOS 13 installed yet, so I can't reproduce this. But nothing about this problem sounds sim-specific. It's almost certainly a general problem that involves scenery.
Assigning to @jonathanolson for investigation, and @kathy-phet for prioritization.
Since this likely affects all sims, labeling as blocks-publication until someone decides otherwise.
Good find @Nancy-Salpepi. Did you notice any other artifacts on Safari? Was this something you noticed often? Can you comment on the severity?
I agree with @pixelzoom, this looks like a scenery issue. It's unclear what sort of impact this may have across sims, so I think it's worth some investigation from @jonathanolson.
It looks like a similar issue was found in Mean: Share & Balance and was patched. Could that be applicable here? https://github.com/phetsims/mean-share-and-balance/issues/122
The workaround in MSaB is brute force, and really ugly. It's putting an expanded rectangle around the things that are leaving artifacts, to make the "redraw" area larger. That's not generally desirable or scalable, and we're in bad shape if that's what we need to do to get clean rendering on Safari. So I'm definitely not doing that until the general issue has gotten some attention.
QA has completed dev test https://github.com/phetsims/qa/issues/847. Further progress is blocked until this issue is addressed. If we wait more than a few days, we'll need to do another dev test, because master will have diverged.
@arouinfar it took a while for me to notice the artifacts on the projector screen, but once I did, it was very obvious to me.
I have definitely seen worse cases with safari--https://github.com/phetsims/gravity-and-orbits/issues/435
I'll look if there's a way to force Safari SVG to force-repaint in a more global way. I don't yet have macOS 13, but I can upgrade to be able to work on this.
It's putting an expanded rectangle around the things that are leaving artifacts, to make the "redraw" area larger.
Wanted to correct this comment. What was actually committed and pushed was a different solution. We identified the guilty path causing the artifacts and set it to renderer: canvas
. Not suggesting that this is a scalable option either. Creating an anti-artifact rectangle actually did not work at all in our case either and renderer: canvas
was the fallback option.
@samreid and I found a hacky way to fix this and wanted to post it here in case it would be of any help to @jonathanolson.
We co-opted the PointerAreas overlay to cover the entire sim screen, and this seemed to trick Safari enough to remove artifacts. You can try it out with this patch:
We confirmed this is fixed on MacOS 13 and Safari 16.1
@pixelzoom - Met with JO and he will work on baking on a workaround in scenery for Safari, so that the work around isn't sim specific. Seems like Safari is not updating SVG drawing quite right.
We will want to test performance on Safari after the fix.
@jonathanolson - Is this something where a bug report to Safari makes sense?? So they may fix it in a future version.
@jonathanolson - Consider for combining with ?allowlinks maintenance release as you do this fix.
@samreid @marlitas the overlay style approach seems to help this issue, but does NOT seem to help the GAO issue. Did you see your solution helping both?
I'm getting this line visual, while paused, while I'm force-changing a rectangle in front (that's very visual).
I'll see if I can stuff the rectangle into the actual SVG elements that are buggy.
Patch for overlay with flashing color:
Forcing a repaint IN every single SVG element still doesn't help:
It's persistent enough that I'm able to see that the GAO issue is related to the arrow.
The line seems to be along the top of the bounding box of the arrow pointing down and to the right.
Adjusting the stroke of the arrow results in the buggy line also changing to that color.
Here's a given path description that's buggy:
<path d="M 457.75108193500130937537 250.67387339438985804918 L 632.18383622661906429130 864.12617889168723195326 L 627.37448216689494984166 865.49369983420137941721 L 638.69107608402407549875 877.87048059960238788335 L 641.80254434606717950373 861.39113700665848227800 L 636.99319028634317874094 862.75865794917285711563 L 462.56043599472542382500 249.30635245187554005497 L 457.75108193500130937537 250.67387339438985804918 Z " style="fill: rgb(50,130,215); stroke: rgb(255,0,0);stroke-miterlimit: 10;"></path>
Fairly minimal example of the Safari GAO issue:
<html>
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title>Arrow bug test</title>
</head>
<body>
<svg focusable="false" width="1502" height="566">
<g transform="matrix(0.84317049365593055299,0.00000000000000000000,0.00000000000000000000,0.84317049365593055299,319.29670724816355686926,0.00000000000000000000)">
<g transform="scale(0.80000000000000004441)">
<path d="M 429.73685877293036128322 152.33263688218951870113 L 475.96967537052415764265 314.78633972164936949412 L 471.16063094948151501740 316.15494914600617448741 L 482.48002585411597920029 328.52916827259900856006 L 485.58776421260949973657 312.04912087293575950753 L 480.77871979156685711132 313.41773029729256450082 L 434.54590319397306075189 150.96402745783271370783 L 429.73685877293036128322 152.33263688218951870113 Z" style="fill: rgb(50,130,215); stroke: rgb(64,64,64);"></path>
</g>
</g>
</svg>
</body>
</html>
is resulting in the buggy case for me:
Adding a "M 0 0" (move to 0,0) at the end of the path seems to work around the issue:
<html>
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<title>Arrow bug test</title>
</head>
<body>
<svg focusable="false" width="1502" height="566">
<g transform="matrix(0.84317049365593055299,0.00000000000000000000,0.00000000000000000000,0.84317049365593055299,319.29670724816355686926,0.00000000000000000000)">
<g transform="scale(0.80000000000000004441)">
<path d="M 429.73685877293036128322 152.33263688218951870113 L 475.96967537052415764265 314.78633972164936949412 L 471.16063094948151501740 316.15494914600617448741 L 482.48002585411597920029 328.52916827259900856006 L 485.58776421260949973657 312.04912087293575950753 L 480.77871979156685711132 313.41773029729256450082 L 434.54590319397306075189 150.96402745783271370783 L 429.73685877293036128322 152.33263688218951870113 Z M 0 0" style="fill: rgb(50,130,215); stroke: rgb(64,64,64);"></path>
</g>
</g>
</svg>
</body>
</html>
Unfortunately... BOTH workarounds are needed (neither affects the other). The overlay workaround may reduce some performance (@KatieWoe can I build some versions or do some dev versions to compare?)
@Nancy-Salpepi can you verify that the above commit fixes this issue?
In case I can work around this in a more performant way (e.g. using Line differently in Safari SVG, with a path instead of a line segment), I'm checking into the reproduction.
Does not update with line color. Cannot isolate with HTML, repaints trigger a clear.
Not related to the lines' stroke
Matches the stroke of the projection path.
Buggy path (conditional):
<path d="M -42 -112 L 42 -134 L 42 134 L -42 112 L -42 -112 Z M0 0" style="fill: rgb(255,255,255); stroke: red;stroke-width: 2;stroke-miterlimit: 10;"></path>
Adjusting things with the path doesn't work. Adjusting line segments to be displayed in SVG with paths with the other workaround isn't effective. Anything that repaints the trapezoid removes the buggy issue.
I think it's best to see the performance impact of the workaround.
@jonathanolson this looks fixed on the mac/iPad. I wasn't able to produce any artifacts on master.
@jonathanolson what's the status here? I see you're still doing performance comparisons in https://github.com/phetsims/qa/issues/856. This is the sole issue blocking RC publication.
@pixelzoom - I'm lifting the blocks publication on this. We will use the workaround that is in place.
@jonathanolson - Leaving this open for you until the maintenance release issue is created.
@jonathanolson @kathy-phet I'm still waiting for update on when/how I should proceed. Your comments above have not answered that question sufficientl. If this is ready to proceed with RC, then someone needs to clearly communicate that, and assign this issue to me so that I know that the work is done.
Let me see if I can summarize, since I want to move forward with this:
Do I have that correct?
@pixelzoom your summary looks accurate to me.
Me too. Thanks for making the common-code issue, for the permenant fix ... which can only come of Safari fixes their bugs.
Further work on this will be done in https://github.com/phetsims/scenery/issues/1502.
Instead of creating new issue https://github.com/phetsims/scenery/issues/1502, we decided to transfer the original issue from geometric-optics-basics to scenery. I'll create a new issue for RC verification in geometric-optics-basics.
As discovered by QA in https://github.com/phetsims/geometric-optics-basics/issues/34#issuecomment-1323679485, the workaround is not working in all cases. I've adjusted the issue labels accordingly.
@kathy-phet said in https://github.com/phetsims/geometric-optics-basics/issues/34#issuecomment-1323923592:
Marking this as "on-hold", as we wait for Safari to address this issue.
There's a chance this could be resolved by OS updates. It looks like it's been reported now by other parties, and seems like it's a bug BELOW webkit.
Coming from MSaB issue https://github.com/phetsims/mean-share-and-balance/issues/122.
Do we know if this has been resolved yet? Unsure if this is something we need to keep an eye out for going into the next round of development for Mean Share and Balance or if that issue can now be closed. @jonathanolson?
I don't know if it's resolved, it's possible we could set something up to check every time a major OS update comes out.
@Nancy-Salpepi can you check and see if this has been resolved with the new OS updates that have come out? It seems like a lift we don't have the resources for right now to do that automatically. If it does look resolved I think this issue can be closed.
@marlitas I hadn't seen them in a while and then last Thursday 2/8 I noticed them again while testing something for @jonathanolson. I am on macOS 14.3 and safari 17.3 (on published and in main).
Here are 2 examples:
@jonathanolson Here is another interesting one with Telugu and safari 17.3.1 seen while testing https://github.com/phetsims/qa/issues/1039 but not in published:
https://github.com/phetsims/scenery/assets/87318828/9bf90029-5fdc-4692-bf74-ed07cad220f2
I saw https://github.com/phetsims/scenery/issues/1503#issuecomment-1944153996 on Waves Intro with Telugu as well. And when the icon was selected the part left behind as a graphic seemed cut off.
Edit: Occurs in Tamil and Kannada locale as well. I suspect it is a general issue
One way to get this is, if a character on the home page is taller than normal, go into the page of that icon, then go to the home page again, and part of that character is left behind as you switch icons.
I have no next steps in this issue. Un-assigning.
Noting the screenshot from the above issue here:
Recent Sim Tests with Artifacts Energy Skate Park - Dec 7, 2022 Atomic Interactions - Dec 7, 2022 Mean: Share and Balance - Dec 12, 2022 Normal Modes (close bottom panel) - Dec 12, 2022 John Travoltage (in preferences dialog when e- are on body) - Dec 12, 2022 Least Squares Regression (in panel with custom)- Dec 12, 2022 Masses and Springs (in panel, in springs) - Dec 12, 2022 Geometric Optics (light scene) - Dec 12, 2022 Friction (in the thermometer) - Dec 22, 2022 Greenhouse (in the energy flux meter) - Jan 6, 2023 Beer's Law Lab/Concentration (detector probe) - Jan 30, 2023 Calculus Grapher 3/20/2023
Test device MacBook Air (m1 chip) and iPad 9th generation
Operating System macOS 13.0 and iPadOS 16
Browser safari 16
Problem description For https://github.com/phetsims/qa/issues/847, in the Light scene, artifacts appear on the screen when the light is moved up and down. I am also seeing this in the published version. I think it is a result of the newest macOS + safari versions because no artifacts occur when I test with my desktop (macOS 10.15.7 + safari 15.6.1). Plus, I also saw artifacts while testing https://github.com/phetsims/qa/issues/848.
Steps to reproduce
Visuals
https://user-images.githubusercontent.com/87318828/199805807-3d65a9cb-34ea-4c1d-ad76-21eb65cb54f6.mp4