phetsims / chipper

Tools for developing and building PhET interactive simulations.
MIT License
11 stars 14 forks source link

How to deal with (0,0) text bounds while sim is running? #768

Open samreid opened 5 years ago

samreid commented 5 years ago

From https://github.com/phetsims/chipper/issues/764

I said:

What if the parent div becomes hidden during the startup process? What if the sim launches, then the parent div becomes hidden, then the PhET sim creates a SCENERY/Text while the parent is hidden, then parent becomes non-hidden. Won't the new SCENERY/Text have incorrect bounds and layout? Should we prevent any simulation activity while bounds cannot be obtained? If so, how could that be done efficiently, without creating SVG nodes and throwing them away every requestAnimationFrame?

@jonathanolson replied:

I tested this, and a timeout of 20ms between displaying and hiding again was able to produce this problem. I don't see any good way of handling this effectively, but I think it's unlikely to be encountered in the wild. Probably the only good way of handling this would be to switch to a different method of acquiring text bounds, but it's highly likely there is another way that gives the same results (so we may see a shift in spacing for text). It's a can of worms that requires a ton of browser testing, but if this becomes more of a problem, it could be looked into.

I agree the primary case is making sure text bounds are available at the beginning of startup, but I think we should decide how to handle this during the sim runtime. Some possibilities:

(a) Throw an error if non-empty text is detected to have (0,0) bounds during sim runtime.
(b) When non-empty text is detected to have (0,0) bounds, use an alternative approach for determining bounds (c) Combine approach (a) with a strategy that preallocates all Text instances, so we can be sure bounds exist during creation (d) When non-empty text is detected to have (0,0) bounds, print something to the console to help us debug this if it comes up in the future

We do not control how clients will hide/show simulation divs, so I think we should be proactive about how to proceed here.

In my opinion, this feature doesn't need to block publication, because we have had this problem for the entire history of our project. Reassigning to @jonathanolson for follow-up discussion.

jonathanolson commented 5 years ago

(a) Throw an error if non-empty text is detected to have (0,0) bounds during sim runtime.

I'm not sure how throwing an error would be helpful for built simulations. What are the advantages of this?

(b) When non-empty text is detected to have (0,0) bounds, use an alternative approach for determining bounds

This sounds like a good fallback to implement. If agreed upon, I'd like to implement this.

(c) Combine approach (a) with a strategy that preallocates all Text instances, so we can be sure bounds exist during creation

It seems like a difficult problem to be able to anticipate every single font (including the size) used by a simulation.

(d) When non-empty text is detected to have (0,0) bounds, print something to the console to help us debug this if it comes up in the future

This also seems generally useful, so we'll see something in the console.

I'd like to hear thoughts, but a combination of (b) and (d) sounds the best to me right now.

samreid commented 5 years ago

but a combination of (b) and (d) sounds the best to me right now.

That sounds good to me too.

jonathanolson commented 5 years ago

It looks like the DOM-based methods aren't working in this situation either (including our built-in ones, and some online like https://stackoverflow.com/questions/10247132/how-can-i-get-the-height-of-the-baseline-of-a-certain-font)

jonathanolson commented 5 years ago

The most reliable fallback was honestly just figuring out a formula for the anticipated bounds of PhetFont and basing things off of that. I've added that above, and it seems to be working acceptably (even if the sim is detecting 0,0s). Additionally, it will print a single warning (so we don't spam things).

@samreid can you review, and is that everything for this issue?

samreid commented 5 years ago

That seems nice, can you please document how you arrived at the heuristics in getNumericSize and something about platforms (what platform(s) they were deduced from or tested on)?