Widget and viewer improvements #1674
Conversation
|
Why not put these events directly on Given these new events, do we still need to allow custom render loops? Or given custom render loops, do we still need these events? CC #648 |
I like that idea. It would be reasonable to move the
These events compose much better than custom render loops. My changes to |
Looks like it.
Sounds good. |
|
@shunter and I have both talked about making changes like this for a while, so this all looks good to me. I agree with @pjcozzi recommendations so I'll wait for those changes to give this a complete look. Eventually we might want to get rid of |
Also Viewer no longer has its own render loop, it just uses the one in `CesiumWidget`.
|
This is ready. In addition to the changes discussed, I was able to remove the custom render loop from |
|
@mramato review and merge when ready. |
| * @param {Scene} scene The scene in which the render error occurred. | ||
| * @param {Error} error The error that occurred during rendering. | ||
| */ | ||
| CesiumWidget.prototype.handleRenderError = function(scene, error) { |
There was a problem hiding this comment.
Is there a reason this is public? We don't call it externally and I can't think of a reason as to why a user would either.
There was a problem hiding this comment.
Not anymore. Originally Viewer's render loop was going to call it, but then I got rid of Viewer's render loop. I'll clean it up.
| if (!defined(time)) { | ||
| time = new JulianDate(); | ||
| } | ||
| try { |
There was a problem hiding this comment.
try/catch causes optimizer bailouts in v8 for the entire function. factor out all this logic into a helper method to keep the scope of the try-catch as small as possible.
This way the try/catch won't disable optimization of the entire function.
| @@ -328,20 +326,6 @@ define([ | |||
| }, | |||
|
|
|||
| /** | |||
| * Gets the event that will be raised when an error is encountered during the default render loop. | |||
| * The widget instance and the generated exception are the only two parameters passed to the event handler. | |||
| * <code>useDefaultRenderLoop</code> will be set to false whenever an exception is generated and must | |||
There was a problem hiding this comment.
Might be a good idea to move this documentation about useDefaultRenderLoop getting set to false somewhere else in this class.
|
Ready. |
…provements Widget and viewer improvements
preRenderandpostRenderevents toCesiumWidgetand mirror them inViewer.ViewerexposeCesiumWidget'srenderErrorevent instead of creating its own.CesiumWidget.onRenderLoopErrortorenderLoopError.CHANGES.mdsays we did this in b23, but we lied.viewerCesiumInspectorMixinuse thepostRenderevent instead of replacing the render loop entirely.