Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealing with dynamically selected mipmap vs image #143

Closed
samreid opened this issue Feb 21, 2020 · 2 comments
Closed

Dealing with dynamically selected mipmap vs image #143

samreid opened this issue Feb 21, 2020 · 2 comments

Comments

@samreid
Copy link
Member

samreid commented Feb 21, 2020

Related to #91 and discovered during phetsims/chipper#874 and tagging for work during phetsims/chipper#850

Elevating priority so we can determine if this is necessary before the migration. Marking blocks sim publication so we can make sure to decide on this issue before next sim publication.

The migration steps don't currently support the dynamic selection of plugin in CoinNodeFactory:

  // images - use mipmaps normally, but use regular images when memory needs to be conserved
  const useMipmaps = !platform.mobileSafari;

  let coinXBackImage;
  // ...

  if ( useMipmaps ) {
    coinXBackImage = require( 'mipmap!EXPRESSION_EXCHANGE/coin-x-back.png' );
    // ..
  }
  else {
    coinXBackImage = require( 'image!EXPRESSION_EXCHANGE/coin-x-back.png' );
    // ...
  }

I'd like to apply a workaround to get this sim running in ES6 modules for now. Some choices:
(a) just use mipmaps for everything, now that we no longer support iPad2 this may be OK
(b) just use images for everything, it is unclear that the mipmaps make any rendering difference in our currently supported browsers, see phetsims/chipper#840. This would reduce memory usage and download size.
(c) load both types statically with import statements (not in an if statement) and then select which one to render with an if statement. This would take even more memory.
(d) change our ES6 module migration and tooling to support this use case.

I'm inclined to go with (a) or (b) because they are a bit easier. I can test (b) myself on a few browsers, so perhaps we'll start with that, but we can change if necessary.

@samreid
Copy link
Member Author

samreid commented Feb 21, 2020

Just image! looked OK on my mac Chrome, Safari and Firefox, so I'll start with that.

@jbphet
Copy link
Contributor

jbphet commented Feb 26, 2020

Looks okay to me too on Chrome on my main screen and spare monitor, and on Firefox on my main screen. I also took a quick look at the history of the file to see if there were any obvious commits related to issues reported about image resolution, and I didn't see any. Let's go with it. Closing.

@jbphet jbphet closed this as completed Feb 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants