phetsims / build-a-molecule

"Build a Molecule" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
8 stars 7 forks source link

3D view of molecule in wrong location and stick model #111

Closed KatieWoe closed 4 years ago

KatieWoe commented 4 years ago

Test device Dell Operating System Win 10 Browser Chrome Problem description For https://github.com/phetsims/QA/issues/459 The 3D view of the model is partially off the screen when it comes up. Changing the window size seems to fix this. Some of the molecules, such as the ball and stick view, look wrong as well.

Visuals ballandstick threedbad

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Build a Molecule‬ URL: https://phet-dev.colorado.edu/html/build-a-molecule/0.0.0-dev.23/phet/build-a-molecule_all_phet.html Version: 0.0.0-dev.23 2019-11-20 23:29:39 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36 Language: en-US Window: 1536x722 Pixel Ratio: 2.5/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}
Denz1994 commented 4 years ago

Regarding the location of the dialog box: Centering only occurs correctly after adjusting the screen bounds. If the molecule Three.3DObjectis removed from the dialog, this isn't a problem, so I think this has to do with an internal layout problem with the object itself, not dialog.

I'm not sure whether this is due to an implementation detail in BAM or Mobius, but I will pair with @jonathanolson to figure out.

ghost commented 4 years ago

Also, a molecule can be off-center:

issue

Denz1994 commented 4 years ago

In regards to the dialog box being centered: The issue is that the content was being added after the dialog was being initialized. Dialog handles this by resizing the dialog box to the newly added content (see resize in Dialog.options). However, the now resized dialog doesn't re-center itself (see layoutStrategy in Dialog.options). @jonathanolson thought we should log an issue about the Dialog behavior.

Solution: Create the content for the Dialog and then initialize the dialog with the content. Don't add it after initialization. This centered the Dialog box on the screen.

The molecules themselves still need a fix (https://github.com/phetsims/build-a-molecule/issues/111#issuecomment-557711710).

Denz1994 commented 4 years ago

The rendering of the molecules in the dialog are now supported via ThreeJS. There is still an issue of positioning as described in https://github.com/phetsims/build-a-molecule/issues/111#issuecomment-557711710. The molecules that are required for the collection boxes have been fixed but a more general solution should be applied here.

Looking at CompleteMolecule.PubChemAtom2 it is constructed with molecule data that supports 2d modeling but not 3d modeling. For molecules like this (borane, thioxoboron, trifluoromethane, etc), we don't currently have a good means of converting that data that syncs well with ThreeJS. We should either look to using different modeling data for the molecules without 3d data or find a better conversion.

Denz1994 commented 4 years ago

Model molecule data has been adjusted in the above commits. This should affect the specified molecules in the collection boxes and all molecules that can be modeled with the 3d dialog but don't have 3d data (of type PubChemAtom2)

@KatieWoe or @arouinfar Can you see if this is still an issue in this dev version? That also means the centering of molecules here https://github.com/phetsims/build-a-molecule/issues/111#issuecomment-557711710.

arouinfar commented 4 years ago

I am unable to reproduce in dev.43, but I'll leave it to @KatieWoe to verify.

KatieWoe commented 4 years ago

Checked several molecules and it is looking pretty good in that dev test

Denz1994 commented 4 years ago

Thanks a lot for the verification. Closing this one.