From 0e08c5e48f5fffa5b4856917e57d96f69d4defdf Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Sat, 22 Feb 2025 02:34:30 -0600 Subject: [PATCH 1/7] Refactor and cleanup in logic related to the rendering --- client/plots/violin.js | 6 +--- client/plots/violin.renderer.js | 53 ++++++++++++++------------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/client/plots/violin.js b/client/plots/violin.js index 696e1da681..13c086b3f7 100644 --- a/client/plots/violin.js +++ b/client/plots/violin.js @@ -272,10 +272,6 @@ class ViolinPlot { const args = this.validateArgs() this.data = await this.app.vocabApi.getViolinPlotData(args) - if (this.settings.plotThickness == undefined) { - const thickness = this.data.plots.length == 1 ? 200 : 150 - this.settings.plotThickness = Math.min(1400 / this.data.plots.length, thickness) - } if (this.data.error) throw this.data.error /* .min @@ -383,7 +379,7 @@ export function getDefaultViolinSettings(app, overrides = {}) { lines: [], unit: 'abs', // abs: absolute scale, log: log scale rowSpace: 5, - plotThickness: undefined, + plotThickness: 100, medianLength: 7, medianThickness: 3, ticks: 20, diff --git a/client/plots/violin.renderer.js b/client/plots/violin.renderer.js index 8503b777c5..7ca50c8a15 100644 --- a/client/plots/violin.renderer.js +++ b/client/plots/violin.renderer.js @@ -61,7 +61,7 @@ export default function setViolinRenderer(self) { for (const [plotIdx, plot] of self.data.plots.entries()) { const violinG = createViolinG(svgData, plot, plotIdx, isH) if (self.opts.mode != 'minimal') renderLabels(t1, t2, violinG, plot, isH, settings, tip) - renderViolinPlot(plot, self, isH, svgData, plotIdx, violinG, imageOffset) + renderViolinPlot(plot, self, isH, svgData, violinG, imageOffset) if (self.config.term.term.type == TermTypes.SINGLECELL_GENE_EXPRESSION) { // is sc data, disable brushing for now because 1) no use 2) avoid bug of listing cells @@ -89,9 +89,8 @@ export default function setViolinRenderer(self) { } } - self.getPlotThickness = function () { - //self.settings.plotThickness may be undefined if loading a state, because main is not executed - return (self.settings.plotThickness || 145) + self.settings.rowSpace + self.getPlotThicknessWithPadding = function () { + return self.settings.plotThickness + self.settings.rowSpace } self.renderPvalueTable = function () { @@ -138,7 +137,7 @@ export default function setViolinRenderer(self) { const rows = self.data.pvalues const isH = this.settings.orientation === 'horizontal' const maxHeight = isH - ? self.getPlotThickness() * this.data.plots.length + ? self.getPlotThicknessWithPadding() * this.data.plots.length + 10 //add axes height : this.settings.svgw + this.config.term.term.name.length renderTable({ rows, @@ -182,18 +181,15 @@ export default function setViolinRenderer(self) { ) const margin = createMargins(labelsize, settings, isH, self.opts.mode == 'minimal') - const plotThickness = self.getPlotThickness() + const plotThickness = self.getPlotThicknessWithPadding() + const width = + margin.left + margin.top + (isH ? settings.svgw : plotThickness * self.data.plots.length + t1.term.name.length) + const height = + margin.bottom + margin.top + (isH ? plotThickness * self.data.plots.length : settings.svgw + t1.term.name.length) + violinSvg - .attr( - 'width', - margin.left + margin.top + (isH ? settings.svgw : plotThickness * self.data.plots.length + t1.term.name.length) - ) - .attr( - 'height', - margin.bottom + - margin.top + - (isH ? plotThickness * self.data.plots.length : settings.svgw + t1.term.name.length) - ) + .attr('width', width) + .attr('height', height) .classed('sjpp-violin-plot', true) .attr('data-testid', 'sja_violin_plot') @@ -260,16 +256,10 @@ export default function setViolinRenderer(self) { // of one plot // adding .5 to plotIdx allows to anchor each plot to the middle point - const violinG = svg.svgG - .append('g') - .datum(plot) - .attr( - 'transform', - isH - ? 'translate(0,' + self.getPlotThickness() * (plotIdx + 0.5) + ')' - : 'translate(' + self.getPlotThickness() * (plotIdx + 0.5) + ',0)' - ) - .attr('class', 'sjpp-violinG') + const height = self.getPlotThicknessWithPadding() + const step = height * (plotIdx + 0.5) + const translate = isH ? `translate(0, ${step}) ` : `translate(${step}, 0)` + const violinG = svg.svgG.append('g').datum(plot).attr('transform', translate).attr('class', 'sjpp-violinG') return violinG } @@ -305,12 +295,13 @@ export default function setViolinRenderer(self) { .attr('transform', isH ? null : 'rotate(-90)') } - function renderViolinPlot(plot, self, isH, svgData, plotIdx, violinG, imageOffset) { - const plotThickness = self.getPlotThickness() - // times 0.45 will leave out 10% as spacing between plots + function renderViolinPlot(plot, self, isH, svgData, violinG, imageOffset) { + const plotThickness = self.settings.plotThickness + // The scale uses half of the plotThickness as the maximum value as the image is symmetrical + // Only one half of the image is computed and the other half is mirrored const wScale = scaleLinear() .domain([plot.density.densityMax, plot.density.densityMin]) - .range([plotThickness * 0.45, 0]) + .range([plotThickness / 2, 0]) let areaBuilder if (isH) { areaBuilder = line() @@ -412,7 +403,7 @@ export default function setViolinRenderer(self) { function renderLines(violinG, isH, lines, svgData) { // render straight lines on plot - const plotThickness = self.settings.plotThickness || 150 //When loading plot from state plotThickness is not initialized + const plotThickness = self.settings.plotThickness violinG.selectAll('.sjpp-vp-line').remove() if (!lines?.length) return From 0696ae406f5d7f15c7448ee1049aa8cdcbd63a45 Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Sat, 22 Feb 2025 04:09:35 -0600 Subject: [PATCH 2/7] Fixes issue #2839 by rendering each plot, getting its size and based on that size positioning the next plot. --- client/plots/violin.renderer.js | 131 ++++++++++++++++---------------- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/client/plots/violin.renderer.js b/client/plots/violin.renderer.js index 7ca50c8a15..bd588f7532 100644 --- a/client/plots/violin.renderer.js +++ b/client/plots/violin.renderer.js @@ -4,7 +4,7 @@ import { curveBasis, line } from 'd3-shape' import { getColors } from '#shared/common.js' import { brushX, brushY } from 'd3-brush' import { renderTable, Menu, getMaxLabelWidth, table2col } from '#dom' -import { rgb } from 'd3' +import { rgb, create } from 'd3' import { format as d3format } from 'd3-format' import { TermTypes } from '#shared/terms.js' @@ -57,11 +57,32 @@ export default function setViolinRenderer(self) { const svgData = renderSvg(t1, self, isH, settings) renderScale(t1, t2, settings, isH, svgData, self) - + let y = self.settings.rowSpace for (const [plotIdx, plot] of self.data.plots.entries()) { - const violinG = createViolinG(svgData, plot, plotIdx, isH) + const plotThickness = self.settings.plotThickness + // The scale uses half of the plotThickness as the maximum value as the image is symmetrical + // Only one half of the image is computed and the other half is mirrored + const wScale = scaleLinear() + .domain([plot.density.densityMax, plot.density.densityMin]) + .range([plotThickness / 2, 0]) + let areaBuilder + //when doing this interpolation, the violin plot will be smoother and some padding may be added + //between the plot and the axis + if (isH) { + areaBuilder = line() + .curve(curveBasis) + .x(d => svgData.axisScale(d.x0)) + .y(d => wScale(d.density)) + } else { + areaBuilder = line() + .curve(curveBasis) + .x(d => wScale(d.density)) + .y(d => svgData.axisScale(d.x0)) + } + //if only one plot pass area builder to calculate the exact height of the plot + const { violinG, height } = renderViolinPlot(svgData, plot, isH, wScale, areaBuilder, y, imageOffset) + y += height if (self.opts.mode != 'minimal') renderLabels(t1, t2, violinG, plot, isH, settings, tip) - renderViolinPlot(plot, self, isH, svgData, violinG, imageOffset) if (self.config.term.term.type == TermTypes.SINGLECELL_GENE_EXPRESSION) { // is sc data, disable brushing for now because 1) no use 2) avoid bug of listing cells @@ -252,16 +273,51 @@ export default function setViolinRenderer(self) { } } - function createViolinG(svg, plot, plotIdx, isH) { + function renderViolinPlot(svgData, plot, isH, wScale, areaBuilder, y, imageOffset) { + const label = plot.label?.split(',')[0] + const catTerm = self.config.term.q.mode == 'discrete' ? self.config.term : self.config.term2 + const category = catTerm?.term.values ? Object.values(catTerm.term.values).find(o => o.label == label) : null + + const color = category?.color ? category.color : self.config.settings.violin.defaultColor + // : plot.divideTwBins + // ? plot.divideTwBins.color + // : self.config.term2 + // ? self.k2c(plotIdx) + // : self.config.settings.violin.defaultColor + if (!plot.color) plot.color = color + if (category && !category.color) category.color = color // of one plot // adding .5 to plotIdx allows to anchor each plot to the middle point + const svg = svgData.svgG + const violinG = svg.append('g').datum(plot).attr('class', 'sjpp-violinG') + renderArea(violinG, plot, areaBuilder) + //render symmetrical violin plot + renderArea(violinG, plot, isH ? areaBuilder.y(d => -wScale(d.density)) : areaBuilder.x(d => -wScale(d.density))) - const height = self.getPlotThicknessWithPadding() - const step = height * (plotIdx + 0.5) - const translate = isH ? `translate(0, ${step}) ` : `translate(${step}, 0)` - const violinG = svg.svgG.append('g').datum(plot).attr('transform', translate).attr('class', 'sjpp-violinG') + renderSymbolImage(self, violinG, plot, isH, imageOffset) + if (self.opts.mode != 'minimal') renderMedian(violinG, isH, plot, svgData, self) + renderLines(violinG, isH, self.config.settings.violin.lines, svgData) + if (self.state.config.value) { + const value = svgData.axisScale(self.state.config.value) + const s = self.config.settings.violin + violinG + .append('line') + .style('stroke', 'black') + .style('stroke-width', s.medianThickness) + .attr('x1', 200) + .attr('x2', 200) + .attr('x1', isH ? value : -s.medianLength) + .attr('x2', isH ? value : s.medianLength) + .attr('y1', isH ? -s.medianLength : value) + .attr('y2', isH ? s.medianLength : value) + } + const rect = violinG.node().getBBox() + let height = isH ? rect.height : rect.width + height += self.settings.rowSpace + const translate = isH ? `translate(0, ${y + height / 2}) ` : `translate(${y + height / 2}, 0)` + violinG.attr('transform', translate) - return violinG + return { violinG, height } } function renderLabels(t1, t2, violinG, plot, isH, settings, tip) { @@ -295,61 +351,6 @@ export default function setViolinRenderer(self) { .attr('transform', isH ? null : 'rotate(-90)') } - function renderViolinPlot(plot, self, isH, svgData, violinG, imageOffset) { - const plotThickness = self.settings.plotThickness - // The scale uses half of the plotThickness as the maximum value as the image is symmetrical - // Only one half of the image is computed and the other half is mirrored - const wScale = scaleLinear() - .domain([plot.density.densityMax, plot.density.densityMin]) - .range([plotThickness / 2, 0]) - let areaBuilder - if (isH) { - areaBuilder = line() - .curve(curveBasis) - .x(d => svgData.axisScale(d.x0)) - .y(d => wScale(d.density)) - } else { - areaBuilder = line() - .curve(curveBasis) - .x(d => wScale(d.density)) - .y(d => svgData.axisScale(d.x0)) - } - - const label = plot.label?.split(',')[0] - const catTerm = self.config.term.q.mode == 'discrete' ? self.config.term : self.config.term2 - const category = catTerm?.term.values ? Object.values(catTerm.term.values).find(o => o.label == label) : null - - const color = category?.color ? category.color : self.config.settings.violin.defaultColor - // : plot.divideTwBins - // ? plot.divideTwBins.color - // : self.config.term2 - // ? self.k2c(plotIdx) - // : self.config.settings.violin.defaultColor - if (!plot.color) plot.color = color - if (category && !category.color) category.color = color - - renderArea(violinG, plot, areaBuilder) - renderArea(violinG, plot, isH ? areaBuilder.y(d => -wScale(d.density)) : areaBuilder.x(d => -wScale(d.density))) - - renderSymbolImage(self, violinG, plot, isH, imageOffset) - if (self.opts.mode != 'minimal') renderMedian(violinG, isH, plot, svgData, self) - renderLines(violinG, isH, self.config.settings.violin.lines, svgData) - if (self.state.config.value) { - const value = svgData.axisScale(self.state.config.value) - const s = self.config.settings.violin - violinG - .append('line') - .style('stroke', 'black') - .style('stroke-width', s.medianThickness) - .attr('x1', 200) - .attr('x2', 200) - .attr('x1', isH ? value : -s.medianLength) - .attr('x2', isH ? value : s.medianLength) - .attr('y1', isH ? -s.medianLength : value) - .attr('y2', isH ? s.medianLength : value) - } - } - function renderArea(violinG, plot, areaBuilder) { if (plot.density.densityMax == 0) return violinG From 86bf722115fff536a04d02cbbb4d0344015ae7d3 Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Tue, 25 Feb 2025 09:55:50 -0600 Subject: [PATCH 3/7] Added option to edit plot padding --- client/plots/violin.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/plots/violin.js b/client/plots/violin.js index 13c086b3f7..401f24be97 100644 --- a/client/plots/violin.js +++ b/client/plots/violin.js @@ -195,6 +195,16 @@ class ViolinPlot { min: 40, debounceInterval: 1000 }, + { + label: 'Plot padding', + type: 'number', + chartType: 'violin', + settingsKey: 'rowSpace', + step: 1, + max: 20, + min: 0, + debounceInterval: 1000 + }, { label: 'Median length', title: 'Length of median', From 953d058b3e0be6541d0a47f7014a1642d8ebe7b4 Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Tue, 25 Feb 2025 13:05:00 -0600 Subject: [PATCH 4/7] Increased default plot thickness --- client/plots/violin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/plots/violin.js b/client/plots/violin.js index 401f24be97..d5ee286ca2 100644 --- a/client/plots/violin.js +++ b/client/plots/violin.js @@ -389,7 +389,7 @@ export function getDefaultViolinSettings(app, overrides = {}) { lines: [], unit: 'abs', // abs: absolute scale, log: log scale rowSpace: 5, - plotThickness: 100, + plotThickness: 150, medianLength: 7, medianThickness: 3, ticks: 20, From c0997504bc53d9466ff24b63db1bb0bee0a9635a Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Tue, 25 Feb 2025 13:29:23 -0600 Subject: [PATCH 5/7] Increased SC plot thickness --- client/plots/singleCellPlot.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/plots/singleCellPlot.js b/client/plots/singleCellPlot.js index 615a03839c..12a135f5c8 100644 --- a/client/plots/singleCellPlot.js +++ b/client/plots/singleCellPlot.js @@ -616,7 +616,7 @@ class singleCellPlot { plots: [ { chartType: 'violin', - settings: { violin: { plotThickness: 50 } }, + settings: { violin: { plotThickness: 90 } }, term: { $id: await digestMessage(`${gene}-${this.state.config.sample}-${this.state.config.experimentID}`), term: { From f8d9b91023a49f5aaf8a20aa7f6a785c06a36864 Mon Sep 17 00:00:00 2001 From: Gavriel Matt Date: Tue, 25 Feb 2025 14:14:40 -0600 Subject: [PATCH 6/7] update default thickness and spacing --- client/plots/violin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/plots/violin.js b/client/plots/violin.js index d5ee286ca2..b7dd1cf1b9 100644 --- a/client/plots/violin.js +++ b/client/plots/violin.js @@ -388,8 +388,8 @@ export function getDefaultViolinSettings(app, overrides = {}) { rightMargin: 50, lines: [], unit: 'abs', // abs: absolute scale, log: log scale - rowSpace: 5, - plotThickness: 150, + rowSpace: 10, + plotThickness: 130, medianLength: 7, medianThickness: 3, ticks: 20, From 657e475cca707a71b5f17786a04f613ee1a9bcd4 Mon Sep 17 00:00:00 2001 From: Airen Zaldivar Date: Tue, 25 Feb 2025 15:29:51 -0600 Subject: [PATCH 7/7] Calculate plot size based on the number of categories. If a plot thickness is specified use it instead --- client/plots/singleCellPlot.js | 1 - client/plots/violin.js | 5 ++--- client/plots/violin.renderer.js | 12 +++++++++--- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/client/plots/singleCellPlot.js b/client/plots/singleCellPlot.js index 12a135f5c8..2ef6611225 100644 --- a/client/plots/singleCellPlot.js +++ b/client/plots/singleCellPlot.js @@ -616,7 +616,6 @@ class singleCellPlot { plots: [ { chartType: 'violin', - settings: { violin: { plotThickness: 90 } }, term: { $id: await digestMessage(`${gene}-${this.state.config.sample}-${this.state.config.experimentID}`), term: { diff --git a/client/plots/violin.js b/client/plots/violin.js index b7dd1cf1b9..36498643fd 100644 --- a/client/plots/violin.js +++ b/client/plots/violin.js @@ -186,12 +186,12 @@ class ViolinPlot { }, { label: 'Plot thickness', - title: 'Thickness of plots, can be between 40 and 200', + title: 'If not specified, the plot thickness is calculated based on the number of categories', type: 'number', chartType: 'violin', settingsKey: 'plotThickness', step: 10, - max: 500, + max: 200, min: 40, debounceInterval: 1000 }, @@ -389,7 +389,6 @@ export function getDefaultViolinSettings(app, overrides = {}) { lines: [], unit: 'abs', // abs: absolute scale, log: log scale rowSpace: 10, - plotThickness: 130, medianLength: 7, medianThickness: 3, ticks: 20, diff --git a/client/plots/violin.renderer.js b/client/plots/violin.renderer.js index bd588f7532..ab1a3d57d2 100644 --- a/client/plots/violin.renderer.js +++ b/client/plots/violin.renderer.js @@ -58,13 +58,13 @@ export default function setViolinRenderer(self) { const svgData = renderSvg(t1, self, isH, settings) renderScale(t1, t2, settings, isH, svgData, self) let y = self.settings.rowSpace + const thickness = self.settings.plotThickness || self.getAutoThickness() for (const [plotIdx, plot] of self.data.plots.entries()) { - const plotThickness = self.settings.plotThickness // The scale uses half of the plotThickness as the maximum value as the image is symmetrical // Only one half of the image is computed and the other half is mirrored const wScale = scaleLinear() .domain([plot.density.densityMax, plot.density.densityMin]) - .range([plotThickness / 2, 0]) + .range([thickness / 2, 0]) let areaBuilder //when doing this interpolation, the violin plot will be smoother and some padding may be added //between the plot and the axis @@ -109,9 +109,15 @@ export default function setViolinRenderer(self) { td2.style('text-align', 'center').text(stat.value ?? 0) } } + self.getAutoThickness = function () { + if (self.data.plots.length === 1) return 150 + const count = self.data.plots.length + return Math.min(130, Math.max(60, 600 / count)) //clamp between 60 and 130 + } self.getPlotThicknessWithPadding = function () { - return self.settings.plotThickness + self.settings.rowSpace + const plotThickness = self.settings.plotThickness || self.getAutoThickness() + return plotThickness + self.settings.rowSpace } self.renderPvalueTable = function () {