Image performance issues

athackst commented 5 years ago

I see items added to the 3D display, but where does the "Image" type show up?

chapulina commented 5 years ago

Once you click + then Image, you get an Image item on the menu, and a floating image display. Which one are you missing?

athackst commented 5 years ago

I don't see the floating image.

chapulina commented 5 years ago

The element is transparent by default, are you sure there are images being published?

It would be good for us to add a placeholder rectangle with some text when there are no images being received.

athackst commented 5 years ago

Ah, it finally showed up.

It now appears to be very far behind and super fast forwarded video.

chapulina commented 5 years ago

Cool, that sounds like a performance issue, I imagine you're also displaying other things like TF and the robot pose too.

jubeira commented 5 years ago

(Related to #9 as well): we will try changing the dependencies' version to use latest changes and improvements in ros3djs and related libraries and try to identify bottlenecks.

jubeira commented 5 years ago

Update: I tried updating the libraries, but I got some conflicts with the versions.

ros3djs was changed to target develop branch, which depends on a different version of threejs than the one used in polymer-ros-rviz.
ros3djs uses roslibjs#0.14.0, but ros-rviz also needs ros-websocket which uses 0.18.0 (see here). Right now, ros-rviz is using a specific hash for ros3djs.

I tried making the versions of the libraries match but I got some Javascript errors coming from the console:

TypeError: THREE.EventDispatcher.prototype.apply is not a function

so I'm clearly missing something out. Perhaps it's not a big deal, but it's not as trivial as changing the version numbers of the dependencies either. I'll dig a bit more about this.

chapulina commented 5 years ago

Upgrading three.js versions is always a tricky task. How about we ticket new issues for bumping each version and tackle them one by one? For threejs, the migration guide is a start, but we'll probably also need to research beyond that.

jubeira commented 5 years ago

Alright, adding some more info here:

When taking a look at the dependencies for ros3djs @ develop branch after installing the dependencies and searching for threejs I see bower_components/ros3djs/bower.json: "threejs": "mrdoob/three.js#r88", while with the currently used ros3djs I see bower_components/ros3djs/bower.json: "threejs": "mrdoob/three.js#r64".
The TypeError is coming from a file in the codebase (STLLoader), and the function is defined in here: https://github.com/osrf/polymer-ros-rviz/blob/0cb36be409dab31501f1d6de2d65a432471f3149/scripts/threejs/three.js#L6666-L6679.

So my guess is that the new threejs required by ros3djs (r88) is redefining the event dispatcher somewhere. I'll open a new issue to keep track of this.

mvollrath commented 5 years ago

I worked on this today, testing with a 1920x1080@30 video stream. At first I couldn't reproduce the visual lag, then I opened the dev tools and saw it. You should know that anytime the dev tools are open this application will lag, because Chrome is generating a pile of debug information every time a WS message comes in.

I was still interested in optimizing, so I tried switching to CBOR transport. Because the data is already jpeg compressed on the backend, going from the binary array to the image was about 50% more expensive than text, because the binary data needed base64 encoding on the client side. Binary encoding reduces wire size by a fair amount, and it would be a superior way to show uncompressed (lossless) images, but it seems like for most use cases the text encoding is fine.

A better approach might be to use web_video_server for images because it does more useful things like resize the image on the backend and not self-destruct when dev tools are open. I'll try that next.

mvollrath commented 5 years ago

CBOR with Blob instead of base64 doesn't freeze up when dev tools are open, for whatever reason, even though Blobbing takes twice as much JavaScript time as loading in the base64 string.

mvollrath commented 5 years ago

Outliers abound, the CBOR path is usually faster :+1:

mvollrath commented 5 years ago

Branch based on forks is here: https://github.com/EndPointCorp/polymer-ros-rviz/tree/image_cbor

PR depends on polymer-ros-rviz's dependencies (ros-websocket and ros-service) updating to CBOR-capable roslib, which is in progress.

chapulina commented 5 years ago

It would be good for us to add a placeholder rectangle with some text when there are no images being received.

Branched this to #15

jubeira commented 5 years ago

The binary path for images was merged in https://github.com/osrf/polymer-ros-rviz/pull/16.

chapulina commented 5 years ago

It now appears to be very far behind and super fast forwarded video.

@athackst , let us know if the latest branch works better for you and we can close this issue. Thanks!

osrf / rvizweb

Image performance issues #8