mozilla / lightbeam-we

Web Extension version of the Firefox Lightbeam add-on
https://addons.mozilla.org/en-GB/firefox/addon/lightbeam/
Mozilla Public License 2.0
179 stars 61 forks source link

[WIP] Prevent ghosting - P1 #175

Closed princiya closed 6 years ago

princiya commented 7 years ago

Issue #147

princiya commented 7 years ago

@jonathanKingston I have used d3.timer which internally uses requestAnimationFrame. There isn't ghosting now, but the repaint is too slow.

There is a related D3 issue to improve performace using web worker - https://github.com/d3/d3/issues/1519.

Reading a bit and trying that out!

princiya commented 7 years ago

As I was debugging, I realised in lightbeam.js we have the redraw() function which calls viz.draw(). This redraw function passes the whole this.websites object to the simulation.

Each time we call viz.draw() we are repainting the whole graph and not letting D3 decide which are the new nodes added dynamically. Maybe this is the reason, I couldn't get ontick handler to work because the graph never stabilised as it grew, as it was trying to repaint the whole graph and not account to just the newly added nodes.

When we first started with D3, we had D3's join methods - enter()&exit().

This example graph has a fair amount of nodes and is a force layout example using canvas. This fits our use case, except there is no dynamic behaviour.

So, I shall try and follow the below approach:

biancadanforth commented 7 years ago

@princiya Thanks for writing your thoughts out for the rest of us!

My understanding of D3 is that when we use the join(), enter() and exit() methods, it is figuring out which nodes to keep the same, add and remove, respectively, so that we don't have to write that logic ourselves in lightbeam.js. When I was messing around with D3 initially using these methods, my guess was that the resetting issue was not that we were passing all the website data every time in, but that the simulation and forces were resetting every time.

Here is an example and a good resource on these methods that I was looking at way back when (you probably have seen them before, but just in case):

Now that we are no longer passing in a websites blob to Lightbeam, it may not be too difficult to update one node at a time, as you suggest, but it would be great to know in general if D3 handles this for us with these methods (I didn't realize we were no longer using them).

Either way I'm sure you'll get it working with the tick event!

biancadanforth commented 7 years ago

As you pointed out in slack, these join, enter and exit methods require DOM selections -- that's probably why all the canvas examples you found when you were just transitioning the visualization from svg to canvas use DOM objects still. Would doing this be a terrible idea @princiya @jonathanKingston ?

The example above uses canvas with the join, enter and exit methods.

princiya commented 7 years ago

@biancadanforth we are still good with the 'objects in memory' approach :)

As far as join methods are concerned, the redraw() function gets the updated data (fp/tp) and we have to pass only this piece to the viz.draw() instead of the whole websites object. We wouldn't need enter() or exit() methods from D3 join. We don't have a case for exit() anyways, that rules it out completely.

biancadanforth commented 7 years ago

Thanks Princiya! Though it should be noted that there are updates where we are not adding a new shape/line to the canvas -- the only example I can think of right now is when a third party site later is linked to as a first party site (#110) -- in this case we want to replace the triangle at that node's location with a circle -- is that possible with this approach?

princiya commented 7 years ago

@biancadanforth Thanks for the above pointer. At the moment, I do not have an answer, but I will keep this in mind.

jonathanKingston commented 6 years ago

@princiya I think we can close this right? I don't see this anymore with your latest fixes.

princiya commented 6 years ago

@jonathanKingston closing this.