Fix fill tozero with undefined value and overlaping of fill area #2979
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -259,25 +259,69 @@ function sceneUpdate(gd, subplot) { | |
| // draw traces in proper order | ||
| scene.draw = function draw() { | ||
| var i; | ||
| var we_must_draw_previous_line_marker = 0; | ||
| for(i = 0; i < scene.count; i++) { | ||
| var we_defer_somes_draw = 0; | ||
| if(scene.fill2d && scene.fillOptions[i] && scene.fillOptions[i].fillmode && scene.fillOptions[i].fillmode === 'tonext') { | ||
| we_defer_somes_draw = 1; | ||
| } else { | ||
| if(we_must_draw_previous_line_marker && scene.line2d && scene.lineOptions[i - 1]) { | ||
| scene.line2d.draw(i - 1); | ||
| } | ||
| if(we_must_draw_previous_line_marker && scene.scatter2d && scene.markerOptions[i - 1]) { | ||
| scene.scatter2d.draw(i - 1); | ||
| } | ||
| if(we_must_draw_previous_line_marker && scene.error2d && scene.errorXOptions[i - 1]) { | ||
| scene.error2d.draw(i - 1); | ||
| } | ||
| if(we_must_draw_previous_line_marker && scene.error2d && scene.errorYOptions[i - 1]) { | ||
| scene.error2d.draw(i - 1 + scene.count); | ||
| } | ||
| we_must_draw_previous_line_marker = 0; | ||
| } | ||
| if(scene.fill2d && scene.fillOptions[i]) { | ||
|
There was a problem hiding this comment. Hmm. Looks like this broke the
The first two traces have
available here -> https://rreusser.github.io/plotly-mock-viewer/#scatter_fill_self_next There was a problem hiding this comment. i pushed a new version . |
||
| scene.fill2d.draw(i); | ||
| } | ||
| // we draw line2d | ||
| if(we_must_draw_previous_line_marker && scene.line2d && scene.lineOptions[i - 1]) { | ||
| scene.line2d.draw(i - 1); | ||
| } | ||
| if(scene.line2d && scene.lineOptions[i]) { | ||
| if(we_defer_somes_draw === 0) { | ||
| scene.line2d.draw(i); | ||
| } | ||
| } | ||
| // we draw error2d | ||
| if(we_must_draw_previous_line_marker && scene.error2d && scene.errorXOptions[i - 1]) { | ||
| scene.error2d.draw(i - 1); | ||
| } | ||
| if(scene.error2d && scene.errorXOptions[i]) { | ||
| scene.error2d.draw(i); | ||
| } | ||
| if(we_must_draw_previous_line_marker && scene.error2d && scene.errorYOptions[i - 1]) { | ||
| scene.error2d.draw(i - 1 + scene.count); | ||
| } | ||
| if(scene.error2d && scene.errorYOptions[i]) { | ||
| scene.error2d.draw(i + scene.count); | ||
| } | ||
| // we draw scatter2d | ||
| if(!scene.selectBatch || !scene.selectBatch[i]) { | ||
| if(we_must_draw_previous_line_marker && scene.scatter2d && scene.markerOptions[i - 1]) { | ||
| scene.scatter2d.draw(i - 1); | ||
| } | ||
| if(scene.scatter2d && scene.markerOptions[i]) { | ||
| if(we_defer_somes_draw === 0) { | ||
| scene.scatter2d.draw(i); | ||
| } | ||
| } | ||
| } | ||
| if(scene.glText[i] && scene.textOptions[i]) { | ||
| scene.glText[i].render(); | ||
| } | ||
| if(we_defer_somes_draw === 1) { | ||
| we_must_draw_previous_line_marker = 1; | ||
| } else { | ||
| we_must_draw_previous_line_marker = 0; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -287,12 +331,6 @@ function sceneUpdate(gd, subplot) { | |
| scene.scatter2d.draw(scene.unselectBatch); | ||
| } | ||
|
|
||
| scene.dirty = false; | ||
| }; | ||
|
|
||
|
|
@@ -419,6 +457,24 @@ function plot(gd, subplot, cdata) { | |
| } | ||
| if(scene.line2d) { | ||
| scene.line2d.update(scene.lineOptions); | ||
| scene.lineOptions = scene.lineOptions.map(function(lineOptions) { | ||
| if(lineOptions && lineOptions.positions) { | ||
ErwanMAS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| var pos = [], srcPos = lineOptions.positions; | ||
|
|
||
| var firstptdef = 0; | ||
| while(isNaN(srcPos[firstptdef]) || isNaN(srcPos[firstptdef + 1])) { | ||
| firstptdef += 2; | ||
| } | ||
| var lastptdef = srcPos.length - 2; | ||
| while(isNaN(srcPos[lastptdef]) || isNaN(srcPos[lastptdef + 1])) { | ||
| lastptdef += -2; | ||
| } | ||
| pos = pos.concat(srcPos.slice(firstptdef, lastptdef + 2)); | ||
| lineOptions.positions = pos; | ||
| } | ||
| return lineOptions; | ||
| }); | ||
| scene.line2d.update(scene.lineOptions); | ||
| } | ||
| if(scene.error2d) { | ||
| var errorBatch = (scene.errorXOptions || []).concat(scene.errorYOptions || []); | ||
|
|
@@ -441,16 +497,38 @@ function plot(gd, subplot, cdata) { | |
| var pos = [], srcPos = (lineOptions && lineOptions.positions) || stash.positions; | ||
|
|
||
| if(trace.fill === 'tozeroy') { | ||
| var firstpdef = 0; | ||
| while(isNaN(srcPos[firstpdef + 1])) { | ||
| firstpdef += 2; | ||
| } | ||
| var lastpdef = srcPos.length - 2; | ||
| while(isNaN(srcPos[lastpdef + 1])) { | ||
| lastpdef += -2; | ||
| } | ||
| if(srcPos[firstpdef + 1] !== 0) { | ||
| pos = [ srcPos[firstpdef], 0 ]; | ||
| } | ||
| pos = pos.concat(srcPos.slice(firstpdef, lastpdef + 2)); | ||
| if(srcPos[lastpdef + 1] !== 0) { | ||
| pos = pos.concat([ srcPos[lastpdef], 0 ]); | ||
| } | ||
| } | ||
| else if(trace.fill === 'tozerox') { | ||
| var firstptdef = 0; | ||
| while(isNaN(srcPos[firstptdef])) { | ||
| firstptdef += 2; | ||
| } | ||
| var lastptdef = srcPos.length - 2; | ||
| while(isNaN(srcPos[lastptdef])) { | ||
| lastptdef += -2; | ||
| } | ||
| if(srcPos[firstptdef] !== 0) { | ||
| pos = [ 0, srcPos[firstptdef + 1] ]; | ||
| } | ||
| pos = pos.concat(srcPos.slice(firstptdef, lastptdef + 2)); | ||
| if(srcPos[lastptdef] !== 0) { | ||
| pos = pos.concat([ 0, srcPos[lastptdef + 1]]); | ||
| } | ||
| } | ||
| else if(trace.fill === 'toself' || trace.fill === 'tonext') { | ||
| pos = []; | ||
|
|
@@ -508,7 +586,7 @@ function plot(gd, subplot, cdata) { | |
| pos = pos.concat(prevLinePos); | ||
| fillOptions.hole = hole; | ||
| } | ||
| fillOptions.fillmode = trace.fill; | ||
| fillOptions.opacity = trace.opacity; | ||
| fillOptions.positions = pos; | ||
|
|
||
|
|
||
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you got the fill part right 🎉
The logic is a little hard to follow; it would be nice to to ♻️ the logic from the SVG version:
plotly.js/src/traces/scatter/plot.js
Lines 95 to 105 in 88d48c5
by using
_nexttraceand_prevtracewhich are defined duringlinkTracescalled in scattergl here.Moreover, looks like the error bars are drawn above the lines, SVG scatter does it the other way around.
Oh well, I'll try out a few things and hopefully that one or two commits will be enough to bring this to the finish line.
Thanks again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ErwanMAS I made two commits on branch
etienne-scattergl-orderingwhich appear to make things work while reusing the SVG scatter logic, See:Do they look ok to you? If so, I'll release them along with your work in next week's
v1.42.0.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etpinard for me this seem to be good . Can you take the last version of https://github.com/plotly/plotly.js/blob/4b2b0d5d34e648e361532fe23f5dbff536792e1c/test/image/mocks/gl2d_scatter_fill_self_next_vs_nogl.json . The new version has now 4 charts .