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

Electrons are pixelated #121

Closed
matthew-blackman opened this issue Mar 26, 2024 · 5 comments
Closed

Electrons are pixelated #121

matthew-blackman opened this issue Mar 26, 2024 · 5 comments

Comments

@matthew-blackman
Copy link
Contributor

matthew-blackman commented Mar 26, 2024

Test device
MacBook Pro M1

Operating System
macOS Monterey - 12.0.1

Browser
Chrome 123.0.6312.59

As part of #103, I noticed that the electron images appear pixelated (see screenshot below). Could this be a similar issue to #113? Seeing the visual improvements to the sim from that issue being resolved, I think this is worth a similar effort.

Screen Shot 2024-03-26 at 12 27 11 PM
@pixelzoom
Copy link
Contributor

Yes, similar to #113 -- electrons are also rendered using scenery Sprites, see ElectronsNode.ts.

Assigning to @jonathanolson, since he's also investigating #113.

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 26, 2024

I added mipmap support using the same approach that @jonathanolson used in #113, with the same scale factors and mipmapBias. See screenshot below.

@jonathanolson @matthew-blackman please review. Last one to review close, if everything is OK.

screenshot_3135

@jonathanolson
Copy link
Contributor

Looks good to me. Minor stylistic recommendation for matrix.rowMajor arguments to be split in rows of 3, so that it visually looks like the matrix representation, e.g.:

this.matrix.rowMajor(
  ELECTRON_INVERSE_SCALE, 0, position.x + ELECTRON_RADIUS,
  0, ELECTRON_INVERSE_SCALE, position.y + ELECTRON_RADIUS,
  0, 0, 1
);

instead of

this.matrix.rowMajor( ELECTRON_INVERSE_SCALE, 0, position.x + ELECTRON_RADIUS, 0,
        ELECTRON_INVERSE_SCALE, position.y + ELECTRON_RADIUS, 0, 0, 1 );

@jonathanolson jonathanolson removed their assignment Mar 27, 2024
@matthew-blackman
Copy link
Contributor Author

This looks great! Nice work all. Closing.

@pixelzoom
Copy link
Contributor

Minor stylistic recommendation for matrix.rowMajor arguments to be split in rows of 3

Good idea. I'll do that.

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

3 participants