dasmoth / dalliance

Interactive web-based genome browser.
http://www.biodalliance.org/
BSD 2-Clause "Simplified" License
226 stars 68 forks source link

Race condition adding tiers #114

Closed Traksewt closed 9 years ago

Traksewt commented 10 years ago

When manually adding tiers with (browser.addTier) there is a race condition that sometimes makes the tier all grey. After I get grey tiers, if I tap the + or - in the zoom slider, it renders correctly.

Sometimes all the tiers are grey, sometimes only a few of them are grey. Usually at least one is grey on the first load. I make sure I add tiers after the browser initialisation callback. When there is a problem, it actually renders the tier properly while loading, and once it is finished loading, it then goes grey.

I'm guessing it is related to the drawing of the grey edges past the edge of the chromosome.

image

and after a tap it works correctly: image

danvk commented 9 years ago

I've also noticed something like this, again typically on the first display. I have two BAM tracks. Typically on the first display, one of them will display the pileup and then immediately flash to gray.

dasmoth commented 9 years ago

I'm currently not having a lot of luck reproducing this. If either of you are able to share a configuration that demonstrates this problem even somewhat consistently, that would be a great help.

@Traksewt: am I right in thinking that you're calling addTier in an event listener attached using addInitListener? Are you doing anything else in your initListeners? (Also, is there a particular reason why you're adding tracks at this point rather than passing an array of the tracks you want directly to the Browser constructor)?

@danvk: are you doing anything related to dynamic track addition?

Traksewt commented 9 years ago

I have reproduced it here: http://jsfiddle.net/zwy6bwrn/3/

As it appears to be a race condition, it doesn't always occur when the track takes a while to load. After hitting this page a few times I get the issue as the results are cached. image

The reason why I can't view the tracks at startup is that I have written some code to dynamically show the tracks depending on the current resolution. i.e. when the user zooms in, then the tracks get displayed. However for this example I just did the minimum needed to see the problem. browser.addInitListener(function() { console.log('browser ready'); datasources.forEach(function(source) { browser.addTier(source); }); browser.setLocation('19', 0, 55000000); console.log('moving to: ' + ['19', 0, 55000000].join(',')); browser.refresh(); });

dasmoth commented 9 years ago

Thanks @Traksewt, can reproduce on my system and am looking into it now.

As a quick solution, you can fix things by calling setLocation before addTier. Also NB that the call to refresh() isn't needed.

dasmoth commented 9 years ago

This was indeed a race. It's only likely to occur if refresh gets called in the same event handler as setLocation (which is sometimes asynchronous, if it needs to fetch sequence information).

This should be fixed on the master branch now. Certainly, your JSFiddle works for me if I change the script tag to //www.biodalliance.org/dev/dalliance-compiled.js.

It's still not clear to me why you're calling refresh yourself -- in general, this probably shouldn't be considered a "public" method. But it's good to be robust just in case some other code triggers a refresh at a bad moment.