Add resize to Renderable.js #374

akaszynski · 2022-04-20T05:56:14Z

Automatically resizes the pythreejs Renderer.

Resolves #362.

pythreejs/_base/renderable.py

codecov-commenter · 2022-04-20T06:01:50Z

Codecov Report

Merging #374 (671f610) into master (a78cb57) will increase coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
+ Coverage   70.86%   70.96%   +0.10%     
==========================================
  Files          23       23              
  Lines         834      837       +3     
==========================================
+ Hits          591      594       +3     
  Misses        243      243

Impacted Files	Coverage Δ
pythreejs/_base/renderable.py	`85.96% <100.00%> (+0.25%)`	⬆️
pythreejs/core/Renderer.py	`82.97% <100.00%> (+0.37%)`	⬆️
pythreejs/renderers/WebGLRenderer.py	`100.00% <100.00%> (ø)`

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

…ythreejs into fix/renderer_resize

akaszynski · 2022-04-21T04:28:27Z

Hi @vidartf,

This is ready for review.

This repo has been a bit light as of late in regards to commits and PR merges. I'm at a point that I'd like to take up maintenance of this repo if possible.

vidartf

Thanks for this! I've added some comments below about specific parts, but the large points of the logic looks correct 👍

vidartf · 2022-04-29T12:22:57Z

pythreejs/core/Renderer.py

    scene = Instance(Scene).tag(sync=True, **widget_serialization)
    camera = Instance(Camera).tag(sync=True, **widget_serialization)
    controls = List(Instance(Controls)).tag(sync=True, **widget_serialization)
    #effect = Instance(Effect, allow_none=True).tag(sync=True, **widget_serialization)
    background = Color('black', allow_none=True).tag(sync=True)
    background_opacity = Float(1.0, min=0.0, max=1.0).tag(sync=True)

-    def __init__(self, scene, camera, controls=None, antialias=False, alpha=False, webgl_version=2, **kwargs):
+    def __init__(self, scene, camera, controls=None, antialias=False, alpha=False, webgl_version=2, auto_resize=False, **kwargs):


Suggested change

def __init__(self, scene, camera, controls=None, antialias=False, alpha=False, webgl_version=2, auto_resize=False, **kwargs):

The users can pass autoResize here, and it will be passed along to the super's init method via the kwargs. The other args that are listed here explicitly are there because:

They are required (scene and camera)

They are hidden traits (name start with an underscore).

The autoResize trait does not need the same logic.

vidartf · 2022-04-29T12:24:20Z

pythreejs/core/Renderer.py

        super(Renderer, self).__init__(
            scene=scene,
            camera=camera,
            controls=controls or [],
            _antialias=antialias,
            _alpha=alpha,
            _webgl_version=webgl_version,
+            auto_resize=False,


Suggested change

auto_resize=False,

vidartf · 2022-04-29T12:24:52Z

pythreejs/core/Renderer.py

            **kwargs)
        link((self, 'width'), (self, '_width'))
        link((self, 'height'), (self, '_height'))
+        self.autoResize = auto_resize


If we pass along the trait to the super's init, we won't need to set it here.

Suggested change

self.autoResize = auto_resize

vidartf · 2022-04-29T12:25:43Z

pythreejs/renderers/WebGLRenderer.py

@@ -18,6 +18,7 @@ class WebGLRenderer(RenderableWidget):

    width = CInt(200)
    height = CInt(200)
+    autoResize = Bool(False)


This is already declared on the base class, so should not need to be redeclared here.

Suggested change

autoResize = Bool(False)

vidartf · 2022-04-29T12:42:04Z

js/src/_base/Renderable.js

@@ -2,6 +2,7 @@ var _ = require('underscore');
 var widgets = require('@jupyter-widgets/base');
 var $ = require('jquery');
 var Promise = require('bluebird');
+var THREE = require('three');


This import isn't used, so is probably not needed here.

Suggested change

var THREE = require('three');

vidartf · 2022-04-29T13:50:56Z

js/src/_base/Renderable.js

+    // Get the size of the container, or if that doesn't exist, the window
+    getSize: function() {
+        try {
+            const { width, height } = this.el.getBoundingClientRect();


I would also use window.getComputedStyle() for both the container (this.el) and the rendered element (this.renderer.domElement and or this.$frozenRenderer if this.isFrozen is true) to substract the margins and paddings from these sizes. That should hopefully avoid the magic "4.4" below, as the actual size of this padding will depend on the context (e.g. the margin is a CSS variable set by the theme in JupyterLab).

vidartf · 2022-04-29T13:57:58Z

js/src/_base/Renderable.js

+        if (width === 0){
+            return;
+        }
+        this.renderer.setSize(width, height - 4.4);  // Seems to grow by 4.4 px on each resize


If you set the size on the model (this.model.set({"_width": width, "_height": height})), then the update would happen automatically via the existing updateSize method, and the sizes would also persist through a freeze/thaw cycle.

vidartf · 2022-04-29T14:06:43Z

js/src/_base/Renderable.js

+            console.log("bounding client size");
+            return [width, height];
+        } catch (error) {
+            // Not using a container for the renderer


I'm curious about this. When does this happen?

akaszynski added 3 commits April 19, 2022 20:40

add requirements to test

e5f1014

ignore venv

bd7494e

add resize

7c63231

akaszynski marked this pull request as draft April 20, 2022 05:56

akaszynski commented Apr 20, 2022

View reviewed changes

pythreejs/_base/renderable.py Outdated Show resolved Hide resolved

Update pythreejs/_base/renderable.py

56f49c8

akaszynski added 3 commits April 20, 2022 10:04

add listener event

ec2a423

revert file changes

6096af6

Merge branch 'fix/renderer_resize' of https://github.com/akaszynski/p…

a0c6420

…ythreejs into fix/renderer_resize

akaszynski marked this pull request as ready for review April 20, 2022 16:07

allow resize in browser

671f610

This was referenced Apr 24, 2022

link_views() not working for pythreejs pyvista/pyvista#2499

Closed

Use threejs for gallery pyvista/pyvista#2517

Closed

vidartf reviewed Apr 29, 2022

View reviewed changes

akaszynski closed this Mar 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resize to Renderable.js #374

Add resize to Renderable.js #374

akaszynski commented Apr 20, 2022 •

edited

Loading

codecov-commenter commented Apr 20, 2022 •

edited

Loading

akaszynski commented Apr 21, 2022

vidartf left a comment

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

vidartf Apr 29, 2022

Add resize to Renderable.js #374

Add resize to Renderable.js #374

Conversation

akaszynski commented Apr 20, 2022 • edited Loading

codecov-commenter commented Apr 20, 2022 • edited Loading

Codecov Report

akaszynski commented Apr 21, 2022

vidartf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akaszynski commented Apr 20, 2022 •

edited

Loading

codecov-commenter commented Apr 20, 2022 •

edited

Loading