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

Performance when dragging the Object has degraded. #286

Closed
pixelzoom opened this issue Jan 6, 2022 · 9 comments
Closed

Performance when dragging the Object has degraded. #286

pixelzoom opened this issue Jan 6, 2022 · 9 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 6, 2022

On my 2013 Mac Pro, performance when dragging the Object has gotten worse, especially noticeable with "Many" rays.

The most likely suspects are #283 and #125. They involved adding additional Nodes for optical axis and rays, and computing clipArea for the Image as the Object is dragged.

But I should verify (by running an older dev version) that performance was OK before changes were made for those issues.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 6, 2022

Performance dropped between 1.1.0-dev.12 (1/4/22) and 1.1.0-dev.13 (today, 1/6/22). Dragging feels laggy, and frame rate reported by ?profiler is lower. There are only a few significant commits between those versions, and they are all related to #125 (rays). So it should be pretty easy to bisect and locate the problem.

@pixelzoom
Copy link
Contributor Author

When I opened this issue, I was reporting the performance on my 2013 Mac Pro. On my 2019 MacBook Pro, I don't see any performance issue. But something has apparently changed on performance-challenged platforms, so I should investigate on my Mac Pro, an older iPad, etc.

@pixelzoom pixelzoom changed the title Performance when dragging the Object is laggy. Performance when dragging the Object has degraded. Jan 11, 2022
@pixelzoom
Copy link
Contributor Author

Performance feels fine on my iPad6. So I'll have to test on my Mac Pro.

There are a lot of dependencies for computing the clipArea:

//  LightRaysForegroundNode.js
Property.multilink(
      [ representationProperty, opticPositionProperty, targetPositionProperty, isVirtualProperty, visibleBoundsProperty ],
      ( representation: Representation, opticPosition: Vector2, targetPosition: Vector2, isVirtual: boolean, visibleBounds: Bounds2 ) => {
 
// OpticalAxisForegroundNode.js
    Property.multilink(
      [ opticPositionProperty, representationProperty, sourceObjectPositionProperty, targetPositionProperty,
        barrierPositionProperty, isVirtualProperty, modelBoundsProperty ],

There are undoubtedly intermediate states, because some of these dependencies are also related. For example, sourceObjectPositionProperty is a dependency of targetPositionProperty.

For LightRaysForegroundNode, it would be more efficient to listen to LightRaysNode.raysProcessedEmitter, then update the clipArea. OpticalAxisForegroundNode could conceivably do the same, though it's a little odd.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 11, 2022

In the above commits, I switched to using LightRaysNode.raysProcessedEmitter instead of a Multilink. When that Emitter fires, the clipAreas are updated. My Mac Pro is not handy at the moment, so I still need to compare performance.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 12, 2022

I've squeezed every bit of optimization that I can out of this, see above commits.

It feels very responsive on my 2019 MacBook Pro, iPad6, and iPhone 11.

On my MacPro: The unbuilt version feels sluggish, the built version feels OK. So I'm still concerned about performance-challenged platform, particularly Chromebooks.

@arouinfar please review on a performance-challenged platform, and let me know what you think. If you don't have access to a performance-challenged platform, please assign this to QA.

To test:

  • Drag the Object while Rays is set to "Many" to exercise the worst-case scenario.
  • Test the unbuilt version in master, test the built version in 1.1.0-dev.14.
  • Compare to 1.1.0-dev.12, the dev version before I started working on this issue.

Our options are:

(1) Live with sluggish performance on older platforms. Computing and setting clipArea is relatively expensive, so doing it while dragging is going to impact responsiveness.

(2) Replace 3D objects in picture frames with 2D objects that can be dragged similarly. The 3D perspective is gratuitous, doesn't add to the experience, doesn't support learning goals that can't be supported with 2D, and was inherited from the Flash version. Switching to 2D would also make several expensive issues (including this one) irrelevant. (This option is what I recommend.)

(3) Investigate a different approach. The only other approach I can think of is to split Objects, Images, and Projection screen each into 2 Nodes -- a foreground Node, and a backgroundNode. I did not go down this path originally because (a) it's more complicated, and (b) the Image Node is like to show a vertical seam because it's transparent. (This is the most expensive option, and is also likely to encounter problems.)

@arouinfar
Copy link
Contributor

@KatieWoe can you please do some performance testing on Chromebook and a slower iPad? @pixelzoom has provided instructions in #286 (comment).

@arouinfar
Copy link
Contributor

Thanks for the write-up @pixelzoom. I've passed this one off to QA since I don't currently have access to lower-performing devices.

@arouinfar arouinfar removed their assignment Jan 18, 2022
@KatieWoe
Copy link
Contributor

Tested on a slower Chromebook. Master was the slowest, with dev.14 being a bit better and dev.12 being the best. Even dev.12 had some lag, but none would make the sim unusable, or even particularly frustrating from what I saw.

@arouinfar
Copy link
Contributor

Thanks @KatieWoe!

@pixelzoom based on this assessment, I am fine with proceeding with option (1):

(1) Live with sluggish performance on older platforms.

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