GMOD / Apollo

Genome annotation editor with a Java Server backend and a Javascript client that runs in a web browser as a JBrowse plugin.
http://genomearchitect.readthedocs.io/
Other
124 stars 85 forks source link

NeatCanvas features fail to display some GFF3 data. #970

Closed hexylena closed 8 years ago

hexylena commented 8 years ago

Just dumping the error information before class starts:

/apollo/jbrowse/src/dojo/promise/instrumentation.js:20 DOMException: Failed to execute 'addColorStop' on 'CanvasGradient': The value provided ('') could not be parsed as a color.(…) "Error: Failed to execute 'addColorStop' on 'CanvasGradient': The value provided ('') could not be parsed as a color.
    at Error (native)
    at declare.box_renderBox [as renderBox] (https://cpt.tamu.edu/apollo/jbrowse/plugins/NeatCanvasFeatures/js/main.js:184:17)
    at declare.renderSegments (https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/FeatureGlyph/Segments.js:68:14)
    at declare.segments_renderFeature [as renderFeature] (https://cpt.tamu.edu/apollo/jbrowse/plugins/NeatCanvasFeatures/js/main.js:78:14)
    at Object.dispatcher.around.advice (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/aspect.js:110:23)
    at target.(anonymous function).dispatcher [as renderFeature] (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/aspect.js:92:39)
    at declare.renderFeature (https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/FeatureGlyph/Gene.js:135:27)
    at Object.dispatcher.around.advice (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/aspect.js:110:23)
    at target.(anonymous function).dispatcher [as renderFeature] (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/aspect.js:92:39)
    at declare.renderFeature (https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/Track/CanvasFeatures.js:934:21)
    ----------------------------------------
    rejected at signalDeferred (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:84:15)
    at signalListener (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:55:5)
    at signalWaiting (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:28:4)
    at resolve (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:192:5)
    at https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/Track/CanvasFeatures.js:584:65
    at https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/Track/CanvasFeatures.js:330:45
    at Object.array.forEach (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/_base/array.js:244:6)
    at https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/Track/CanvasFeatures.js:329:32
    at runFactory (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/dojo.js:1094:43)
    at execModule (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/dojo.js:1223:5)
    ----------------------------------------
Error
    at then.promise.then (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:252:24)
    at https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/View/Track/CanvasFeatures.js:601:53
    at signalListener (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:37:21)
    at Promise.then.promise.then (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:258:5)
    at NCList.iterate (https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/Store/NCList.js:219:14)
    at declare._getFeatures (https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/Store/SeqFeature/NCList.js:260:29)
    at https://cpt.tamu.edu/apollo/jbrowse/src/JBrowse/Store/SeqFeature/NCList.js:236:23
    at signalListener (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:37:21)
    at signalWaiting (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:28:4)
    at resolve (https://cpt.tamu.edu/apollo/jbrowse/src/dojo/Deferred.js:192:5)"

Gff3 Looks something like this

##gff-version 3
##sequence-region E3_SHartman 1 111906
E3_SHartman progressiveMauve    match   2810    79687   .   -   .   ID=m_2_0_NC_010583;Target=NC_010583
E3_SHartman progressiveMauve    match_part  2810    2852    41.8604651163   +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3057    3090    94.1176470588   +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3098    3108    72.7272727273   +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3109    3208    84  +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3209    3308    94  +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3309    3397    94.3820224719   +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3398    3476    89.8734177215   +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3477    3515    100 +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3516    3615    81  +   .   Parent=m_2_0_NC_010583
E3_SHartman progressiveMauve    match_part  3616    3705    97.7777777778   +   .   Parent=m_2_0_NC_010583

trackList.json

      {
         "label" : "9d87842d209bd730f0d9741b1cdb3735_0",
         "urlTemplate" : "tracks/9d87842d209bd730f0d9741b1cdb3735_0/{refseq}/trackData.json",
         "key" : "Convert XMFA to gapped GFF3 on data 30 and data 31",
         "type" : "JBrowse/View/Track/CanvasFeatures",
         "style" : {
            "color" : "    function(feature, variableName, glyphObject, track) {        var search_up = function self(sf, attr){            if(sf.get(attr) !== undefined){                return sf.get(attr);            }            if(sf.parent() === undefined) {                return;            }else{                return self(sf.parent(), attr);            }        };        var search_down = function self(sf, attr){            if(sf.get(attr) !== undefined){                return sf.get(attr);            }            if(sf.children() === undefined) {                return;            }else{                var kids = sf.children();                for(var child_idx in kids){                    var x = self(kids[child_idx], attr);                    if(x !== undefined){                        return x;                    }                }                return;            }        };        var color = (undefined || search_up(feature, 'color') || search_down(feature, 'color') || '#cab2d6');        var score = (search_up(feature, 'score') || search_down(feature, 'score'));                    var opacity = (score - (1.0)) / ((100.0) - (1.0));                var result = /^#?([a-f\\d]{2})([a-f\\d]{2})([a-f\\d]{2})$/i.exec(color);        var red = parseInt(result[1], 16);        var green = parseInt(result[2], 16);        var blue = parseInt(result[3], 16);        if(isNaN(opacity) || opacity < 0){ opacity = 0; }        return 'rgba(' + red + ',' + green + ',' + blue + ',' + opacity + ')';    }    ",
            "label" : "name,id",
            "description" : "note,description",
            "className" : "feature"
         },
         "compress" : 0,
         "trackType" : "JBrowse/View/Track/CanvasFeatures",
         "category" : "Comparative Genomics",
         "glyph" : "JBrowse/View/FeatureGlyph/Segments",
         "storeClass" : "JBrowse/Store/SeqFeature/NCList"
      },
nathandunn commented 8 years ago

@erasche Did you test this against the master JBrowse? I removed the gradients in this pull request:

https://github.com/GMOD/jbrowse/pull/721

However, make sure that it is reading your colors properly. I didn't check for that. Hopefully we can coordinate a 1.12.2 JBrowse / 2.0.3 JBrowse release.

hexylena commented 8 years ago

@nathandunn Didn't test elsewhere, just saw this pop up before had to give a short talk.

As for which version, I don't completely know.

This was tested on version 076f3a7227f9a9d061724fb23939339122ca9648 of apollo, which used whatever the default behaviour was for including JBrowse.

nathandunn commented 8 years ago

Right, but whatever you have in apollo-config.groovy will over-ride what is in Config.groovy. You’ll what to do a master pull so that it uses bower to install jbrowse.

The jbrowse version of your apollo-config.groovy can be commented out (or you can just copy it). What you need is this:

git { url= "https://github.com/GMOD/jbrowse" // tag = "1.12.1-release" branch = "master" alwaysPull = true alwaysRecheck = true }

Nathan

On Apr 7, 2016, at 10:46 AM, Eric Rasche notifications@github.com wrote:

@nathandunn https://github.com/nathandunn I don't completely know.

This was tested on version 076f3a7 https://github.com/GMOD/Apollo/commit/076f3a7227f9a9d061724fb23939339122ca9648 of apollo, which used whatever the default behaviour was for including JBrowse.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/GMOD/Apollo/issues/970#issuecomment-207022949

hexylena commented 8 years ago
  1. apr. 2016 6.00 p.m. skrev "Nathan Dunn" notifications@github.com:

    Right, but whatever you have in apollo-config.groovy will over-ride what is in Config.groovy. You’ll what to do a master pull so that it uses bower to install jbrowse.

Sorry, was just explaining the particular case I saw this bug--my apollo config did not mention it at all as I was building from the sample-docker-config.groovy

The jbrowse version of your apollo-config.groovy can be commented out (or you can just copy it). What you need is this:

I'll test soon with an updated image and let you know!

git { url= "https://github.com/GMOD/jbrowse" // tag = "1.12.1-release" branch = "master" alwaysPull = true alwaysRecheck = true }

Nathan

On Apr 7, 2016, at 10:46 AM, Eric Rasche notifications@github.com wrote:

@nathandunn https://github.com/nathandunn I don't completely know.

This was tested on version 076f3a7 < https://github.com/GMOD/Apollo/commit/076f3a7227f9a9d061724fb23939339122ca9648> of apollo, which used whatever the default behaviour was for including JBrowse.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub < https://github.com/GMOD/Apollo/issues/970#issuecomment-207022949>

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

nathandunn commented 8 years ago

@erasche Any luck?

hexylena commented 8 years ago

Soonest I'll get to reproduce is monday, sorry

nathandunn commented 8 years ago

@erasche No worries. No rush, just wanted to make sure you hadn't forgotten to post results. We'll be awhile with this new release.

cmdcolin commented 8 years ago

It looks like the style->color callback is returning an empty string somehow. Nevertheless, there are lots of workarounds and lessons to learn from this bug though

1) The gradient is now gone on jbrowse, so updating to latest jbrowse would fix (addColorStop function from error message was implying gradient trying to be used) 2) We can also comment out NeatCanvasFeatures in the Apollo config file as workaround 3) I would recommend that we maybe not turn on NeatFeatures by default in Apollo. It is not turned on by default in JBrowse either. 4) I think the NeatFeatures should be a tracktype instead code that hijacks every drawing function. That way it can be enabled for specific tracks, and it makes it less error prone.

enuggetry commented 8 years ago

"4) I think the NeatFeatures should be a tracktype instead code that hijacks every drawing function. That way it can be enabled for specific tracks, and it makes it less error prone."

This method was done as a means to address the fact that dragableHTMLfeatures renders the elements different than HTMLfeatures. I had originally implemented it "inband" but it only worked for HTMLfeatures.

There is mechanism to enable per-track: https://github.com/GMOD/jbrowse/tree/master/plugins/NeatHTMLFeatures

If there is any interest, I could work on adapting the intron rendering specifically for draggableHTMLFeatures.

But, I would like to add back the gradient option for the jbrowse release (when I get around to it). It's still a nice out-of-box experience and looks good in print.

On Fri, Apr 8, 2016 at 8:48 AM, Colin Diesh notifications@github.com wrote:

It looks like the style->color callback is returning an empty string somehow. Nevertheless, there are lots of workarounds and lessons to learn from this bug though

1) The gradient is now gone on jbrowse, so updating to latest jbrowse would fix (addColorStop function from error message was implying gradient trying to be used) 2) We can also comment out NeatCanvasFeatures in the Apollo config file as workaround 3) I would recommend that we maybe not turn on NeatFeatures by default in Apollo. It is not turned on by default in JBrowse either. 4) I think the NeatFeatures should be a tracktype instead code that hijacks every drawing function. That way it can be enabled for specific tracks, and it makes it less error prone.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/GMOD/Apollo/issues/970#issuecomment-207489032

nathandunn commented 8 years ago

WRT to 3/4) I think the consensus was that we wanted that to be the default for Apollo. I have no opposition to it becoming a track type, as well, but it should be the default for any Genomic Feature. Longer-term I want to see if its possible to ditch HTMLFeatures / NeatFeatures in favor of SVGFeatures / NeatFeatures and render everything natively and more efficiently.

WRT to gradients, yeah, I think it would be a good idea to add it back as an option per track (though not default . . . or if default, it can somehow inherit the colors), which is why I only commented it out.

cmdcolin commented 8 years ago

I want to see if its possible to ditch HTMLFeatures / NeatFeatures in favor of SVGFeatures / NeatFeatures and render everything natively and more efficiently.

I'm not sure I understand the efficient/speed claims. SVG can enable cool things, but it is in most applications slower. Maybe you are not referring to canvas either, but there are lots of documented cases of platforms switching from SVG to canvas for better optimization purposes. pileup.js http://www.hammerlab.org/2015/10/13/svg-canvas-the-pileup-js-journey/ and cytoscape.js https://groups.google.com/forum/#!topic/cytoscape-discuss/bEgh67aw1BI are good examples of platforms that made that switch. I think SVG obviously has areas where it enables cool things, but it doesn't solve everything.

Also, as far as making it the default, my feeling is that people spend a lot of time making their genome browsers look very customized for their tracks. @erasche definitely did for the BLAST tracks for example. I think that overriding their styles is problematic.

nathandunn commented 8 years ago

So, there are a couple of points here:

1 - the current NeatFeatures draws SVG elements over HTMLFeatures after a milestone so it does some kind of weird jaggediness. It would be much more sense to add the SVG plugin over an SVGFeature track. 2 - SVG is slower than Canvas. If you don't require easy interaction or draggability, Canvas is probably going to be your better bet. However, once you start wanting to do multiple layers and easily attach elements SVG makes more sense.
3 - SVG is faster than DIVs (what I'm arguing here): http://stackoverflow.com/a/5883032/1739366

I'm not proposing never using Canvas, but I think that SVG is more extensible for the more interactive tracks. Obviously we need to set that up to be pluggable which is easy in Apollo (just remove those plugins in the config and add your own). I don't think the plugins are included in the tracks by default in JBrowse.

nathandunn commented 8 years ago

Just wanted to note that we're not going to remove any of the current HTMLFeature tracks, I just want to move the defaults to SVG. Hopefully, though any relevant function / styling in HTMLFeatures (or DraggableHTMLFeatures) can be ported over directly to the newer SVG track type. WRT to Canvas, I think that something like paper.js might make sense syooirtubg drag and drop and layers, but that is a ways away: http://paperjs.org/examples/hit-testing/

cmdcolin commented 8 years ago

There are some other rendering issues with the NeatHTMLFeatures even if it is not throwing obvious javascript errors

For example, on the BRCA example on the Apollo sample browser, the genes end up looking very strange and/or blank

http://icebox.lbl.gov/Apollo-staging/jbrowse/index.html?loc=chr17%3A43091114..43091247&organism=211020&tracks=DNA%2CAnnotations%2CGenBank%20TopLevel%20MRNA%2CMyVariant.info%2CMyVariant.info%20clinvar%2CCanvas-MRNA%20-%20GenBank%20TopLevel%2Cdbsnps%2Cclinvar&highlight=

screenshot-icebox lbl gov 2016-04-09 17-37-26

screenshot-icebox lbl gov 2016-04-13 15-16-32

It seems to depend on zoom level

cmdcolin commented 8 years ago

Sorry my last post refers to NeatHTMLFeatures

nathandunn commented 8 years ago

@erasche Is the initial problem still a problem? If so, please re-open it.

You might need to update apollo and then do an "./apollo clean-all" so that it properly pulls in changes, but it should be fixed.

The problem that @cmdcolin alluded to I added here: https://github.com/GMOD/jbrowse/issues/743

hexylena commented 8 years ago

Tested with gmod/apollo:stable and this looks to be fixed.