Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for numeric font weight #6990

Merged
merged 33 commits into from
May 8, 2024
Merged

Add support for numeric font weight #6990

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
37dee48
Revert "no integer weight"
archmoj May 3, 2024
3212866
also drop font-weight 400 when exporting to SVG
archmoj May 3, 2024
448eb80
test scatter font weight
archmoj May 3, 2024
3a3f458
test bar font weight
archmoj May 3, 2024
15b25a5
use numeric font-weight in mocks
archmoj May 3, 2024
be24a14
skip validating font-weight-bar mock
archmoj May 3, 2024
d8424a3
draft log
archmoj May 6, 2024
353efc4
numeric text weight in gl3d
archmoj May 6, 2024
abb3fc8
link to include extra valid font-weight options in css-font and gl-text
archmoj May 6, 2024
8266a6d
test numeric font-weight values in scattergl pipeline
archmoj May 6, 2024
46e6b27
Revert "link to include extra valid font-weight options in css-font a…
archmoj May 6, 2024
3e4942a
fall back for unsupported font-weight values
archmoj May 6, 2024
72044b5
scattergl numeric font-weight render using bold/normal fallback
archmoj May 6, 2024
4d52885
correct mock name
archmoj May 6, 2024
b92ef23
improve scattergl test
archmoj May 6, 2024
b125396
test numeric weight in gl2d axis text
archmoj May 6, 2024
54005b9
convert typed array spec in integer, number, color, etc
archmoj May 6, 2024
09f4dd3
handle typed arrays in scatter3d
archmoj May 6, 2024
f67b40c
handle typed arrays in gl-axes3d
archmoj May 6, 2024
99162e5
handle numeric font weight in mapbox supported fonts
archmoj May 6, 2024
990fa8d
fix toSVG using weight: 400
archmoj May 6, 2024
f3c0356
improve mapbox text weight and italic handling
archmoj May 6, 2024
a5cc7f8
fix support of Open Sans Extrabold fonts
archmoj May 7, 2024
63824c1
test metropolis fonts on mapbox
archmoj May 7, 2024
190aef1
test italic Metropolis fonts
archmoj May 7, 2024
091e7d3
add weight test for open sans fonts - duplicate simple open-sans base…
archmoj May 7, 2024
82de3ff
add weight test for metropolis fonts - duplicate simple metropolis ba…
archmoj May 7, 2024
59779f3
Revert "test italic Metropolis fonts"
archmoj May 7, 2024
f6fcbd7
test numeric weights on axes 3d
archmoj May 8, 2024
10f477f
bump gl-axes3d and gl-scatter3d
archmoj May 8, 2024
82863fd
refactor src/traces/scattergl/convert.js
archmoj May 8, 2024
bff00ac
improve font family checks
archmoj May 8, 2024
48d057f
add comment
archmoj May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/6990_add.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Add support for numeric text font `weight` [[#6990](https://github.com/plotly/plotly.js/pull/6990)]
15 changes: 14 additions & 1 deletion src/lib/coerce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ exports.valObjectMeta = {
requiredOpts: [],
otherOpts: ['dflt', 'min', 'max', 'arrayOk'],
coerceFunction: function(v, propOut, dflt, opts) {
if(isTypedArraySpec(v)) v = decodeTypedArraySpec(v);
archmoj marked this conversation as resolved.
Show resolved Hide resolved

if(!isNumeric(v) ||
(opts.min !== undefined && v < opts.min) ||
(opts.max !== undefined && v > opts.max)) {
Expand All @@ -114,8 +116,15 @@ exports.valObjectMeta = {
'are coerced to the `dflt`.'
].join(' '),
requiredOpts: [],
otherOpts: ['dflt', 'min', 'max', 'arrayOk'],
otherOpts: ['dflt', 'min', 'max', 'arrayOk', 'extras'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archmoj Is this adding a new functionality to coerce, where numeric (or other) properties can also be configured to accept a list of 'extra' values?

That's pretty cool -- is there anywhere we should document that internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.
Yes that's true.
We have other types that support the extras option so I think it's clear internally.
But on plotly.py we need to see if this changes would be reflected properly by codegen.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

Makes me think, it would be awesome if we could build checking the Plotly.py codegen into the CI process. It would have to produce some kind of output that's easy to verify. It could run only if the schema.json has changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add a line in the docstring here explaining extras for integers (like the explanation here for flaglist)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For flag lists combination mean something. That's why it is commented.

coerceFunction: function(v, propOut, dflt, opts) {
if((opts.extras || []).indexOf(v) !== -1) {
propOut.set(v);
return;
}

if(isTypedArraySpec(v)) v = decodeTypedArraySpec(v);
archmoj marked this conversation as resolved.
Show resolved Hide resolved

if(v % 1 || !isNumeric(v) ||
(opts.min !== undefined && v < opts.min) ||
(opts.max !== undefined && v > opts.max)) {
Expand Down Expand Up @@ -156,6 +165,8 @@ exports.valObjectMeta = {
requiredOpts: [],
otherOpts: ['dflt', 'arrayOk'],
coerceFunction: function(v, propOut, dflt) {
if(isTypedArraySpec(v)) v = decodeTypedArraySpec(v);
archmoj marked this conversation as resolved.
Show resolved Hide resolved

if(tinycolor(v).isValid()) propOut.set(v);
else propOut.set(dflt);
}
Expand Down Expand Up @@ -198,6 +209,8 @@ exports.valObjectMeta = {
requiredOpts: [],
otherOpts: ['dflt', 'arrayOk'],
coerceFunction: function(v, propOut, dflt) {
if(isTypedArraySpec(v)) v = decodeTypedArraySpec(v);
archmoj marked this conversation as resolved.
Show resolved Hide resolved

if(v === 'auto') propOut.set('auto');
else if(!isNumeric(v)) propOut.set(dflt);
else propOut.set(modHalf(+v, 360));
Expand Down
31 changes: 22 additions & 9 deletions src/plots/font_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ module.exports = function(opts) {
var editType = opts.editType;
var colorEditType = opts.colorEditType;
if(colorEditType === undefined) colorEditType = editType;

var weight = {
editType: editType,
valType: 'integer',
min: 1,
max: 1000,
extras: ['normal', 'bold'],
dflt: 'normal',
description: [
'Sets the weight (or boldness) of the font.'
].join(' ')
};

if(opts.noNumericWeightValues) {
weight.valType = 'enumerated';
weight.values = weight.extras;
weight.extras = undefined;
weight.min = undefined;
weight.max = undefined;
}

var attrs = {
family: {
valType: 'string',
Expand Down Expand Up @@ -49,15 +70,7 @@ module.exports = function(opts) {
editType: colorEditType
},

weight: {
editType: editType,
valType: 'enumerated',
values: ['normal', 'bold'],
dflt: 'normal',
description: [
'Sets the weight (or boldness) of the font.'
].join(' ')
},
weight: weight,

style: {
editType: editType,
Expand Down
2 changes: 1 addition & 1 deletion src/snapshot/tosvg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ module.exports = function toSVG(gd, format, scale) {

// Drop normal font-weight, font-style and font-variant to reduce the size
var fw = this.style.fontWeight;
if(fw && fw === 'normal') {
if(fw && (fw === 'normal' || fw === 400)) { // font-weight 400 is similar to normal
txt.style('font-weight', undefined);
}
var fs = this.style.fontStyle;
Expand Down
1 change: 1 addition & 0 deletions src/traces/scattergl/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var attrs = module.exports = overrideAll({
editType: 'calc',
colorEditType: 'style',
arrayOk: true,
noNumericWeightValues: true,
variantValues: ['normal', 'small-caps'],
description: 'Sets the text font.'
}),
Expand Down
12 changes: 9 additions & 3 deletions src/traces/scattergl/convert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ function convertTextStyle(gd, trace) {
if(
isArrayOrTypedArray(tfs) ||
Array.isArray(tff) ||
Array.isArray(tfw) ||
Lib.isArrayOrTypedArray(tfw) ||
archmoj marked this conversation as resolved.
Show resolved Hide resolved
Array.isArray(tfy) ||
Array.isArray(tfv)
) {
Expand All @@ -207,7 +207,7 @@ function convertTextStyle(gd, trace) {
) * plotGlPixelRatio;

fonti.family = Array.isArray(tff) ? tff[i] : tff;
fonti.weight = Array.isArray(tfw) ? tfw[i] : tfw;
fonti.weight = weightFallBack(Lib.isArrayOrTypedArray(tfw) ? tfw[i] : tfw);
fonti.style = Array.isArray(tfy) ? tfy[i] : tfy;
fonti.variant = Array.isArray(tfv) ? tfv[i] : tfv;
}
Expand All @@ -216,7 +216,7 @@ function convertTextStyle(gd, trace) {
optsOut.font = {
size: tfs * plotGlPixelRatio,
family: tff,
weight: tfw,
weight: weightFallBack(tfw),
Copy link
Contributor

@emilykl emilykl May 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed if we are already disallowing numeric weight values at the schema level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question.
This is for scattergl with limited support of weight noting that for arrayOk attributes we do not check every element at the supply defaults step which could result in performance issues during interactions with the plot.
As a result this fall back is needed to map unsupported values in arrays to supported options.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, thank you. Could you perhaps add that as a comment above the function definition?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 48d057f.

style: tfy,
variant: tfv
};
Expand All @@ -225,6 +225,12 @@ function convertTextStyle(gd, trace) {
return optsOut;
}

function weightFallBack(w) {
if(w <= 1000) {
return w > 500 ? 'bold' : 'normal';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the threshold be 400?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

500 is Medium looks very similar to Regular.
IMHO it's debatable between 450/500 to 550.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, ok, I guess if the only options are normal or bold we have to draw the line somewhere. Maybe just add a comment explaining why 500 is chosen as the threshold.

}
return w;
}

function convertMarkerStyle(gd, trace) {
var count = trace._length;
Expand Down
60 changes: 41 additions & 19 deletions stackgl_modules/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11905,6 +11905,17 @@ var identity = new Float32Array([
0, 0, 1, 0,
0, 0, 0, 1])

var ab = ArrayBuffer
var dv = DataView

function isTypedArray(a) {
return ab.isView(a) && !(a instanceof dv)
}

function isArrayOrTypedArray(a) {
return Array.isArray(a) || isTypedArray(a)
}

function copyVec3(a, b) {
a[0] = b[0]
a[1] = b[1]
Expand Down Expand Up @@ -11992,8 +12003,8 @@ proto.update = function(options) {
var opt = options[name]
var prev = this[name]
var next
if(nest ? (Array.isArray(opt) && Array.isArray(opt[0])) :
Array.isArray(opt) ) {
if(nest ? (isArrayOrTypedArray(opt) && isArrayOrTypedArray(opt[0])) :
isArrayOrTypedArray(opt) ) {
this[name] = next = [ cons(opt[0]), cons(opt[1]), cons(opt[2]) ]
} else {
this[name] = next = [ cons(opt), cons(opt), cons(opt) ]
Expand All @@ -12011,7 +12022,7 @@ proto.update = function(options) {
var BOOLEAN = parseOption.bind(this, false, Boolean)
var STRING = parseOption.bind(this, false, String)
var COLOR = parseOption.bind(this, true, function(v) {
if(Array.isArray(v)) {
if(isArrayOrTypedArray(v)) {
if(v.length === 3) {
return [ +v[0], +v[1], +v[2], 1.0 ]
} else if(v.length === 4) {
Expand Down Expand Up @@ -21678,6 +21689,17 @@ var IDENTITY = [1,0,0,0,
0,0,1,0,
0,0,0,1]

var ab = ArrayBuffer
var dv = DataView

function isTypedArray(a) {
return ab.isView(a) && !(a instanceof dv)
}

function isArrayOrTypedArray(a) {
return Array.isArray(a) || isTypedArray(a)
}

module.exports = createPointCloud

function transformMat4(x, m) {
Expand Down Expand Up @@ -22065,7 +22087,7 @@ function get_glyphData(glyphs, index, font, pixelRatio) {
var str

// use the data if presented in an array
if(Array.isArray(glyphs)) {
if(isArrayOrTypedArray(glyphs)) {
if(index < glyphs.length) {
str = glyphs[index]
} else {
Expand All @@ -22086,19 +22108,19 @@ function get_glyphData(glyphs, index, font, pixelRatio) {
if(!font) font = {}

var family = font.family
if(Array.isArray(family)) family = family[index]
if(isArrayOrTypedArray(family)) family = family[index]
if(!family) family = "normal"

var weight = font.weight
if(Array.isArray(weight)) weight = weight[index]
if(isArrayOrTypedArray(weight)) weight = weight[index]
if(!weight) weight = "normal"

var style = font.style
if(Array.isArray(style)) style = style[index]
if(isArrayOrTypedArray(style)) style = style[index]
if(!style) style = "normal"

var variant = font.variant
if(Array.isArray(variant)) variant = variant[index]
if(isArrayOrTypedArray(variant)) variant = variant[index]
if(!variant) variant = "normal"

var glyph = getGlyph(str, {
Expand Down Expand Up @@ -22133,15 +22155,15 @@ proto.update = function(options) {
this.lineWidth = options.lineWidth
}
if('project' in options) {
if(Array.isArray(options.project)) {
if(isArrayOrTypedArray(options.project)) {
this.axesProject = options.project
} else {
var v = !!options.project
this.axesProject = [v,v,v]
}
}
if('projectScale' in options) {
if(Array.isArray(options.projectScale)) {
if(isArrayOrTypedArray(options.projectScale)) {
this.projectScale = options.projectScale.slice()
} else {
var s = +options.projectScale
Expand All @@ -22151,7 +22173,7 @@ proto.update = function(options) {

this.projectHasAlpha = false // default to no transparent draw
if('projectOpacity' in options) {
if(Array.isArray(options.projectOpacity)) {
if(isArrayOrTypedArray(options.projectOpacity)) {
this.projectOpacity = options.projectOpacity.slice()
} else {
var s = +options.projectOpacity
Expand Down Expand Up @@ -22262,8 +22284,8 @@ proto.update = function(options) {
var color = [0,0,0,1]
var lineColor = [0,0,0,1]

var isColorArray = Array.isArray(colors) && Array.isArray(colors[0])
var isLineColorArray = Array.isArray(lineColors) && Array.isArray(lineColors[0])
var isColorArray = isArrayOrTypedArray(colors) && isArrayOrTypedArray(colors[0])
var isLineColorArray = isArrayOrTypedArray(lineColors) && isArrayOrTypedArray(lineColors[0])

fill_loop:
for(var i=0; i<numPoints; ++i) {
Expand All @@ -22289,7 +22311,7 @@ proto.update = function(options) {

//Get color
if(!glyphVisible) color = [1,1,1,0]
else if(Array.isArray(colors)) {
else if(isArrayOrTypedArray(colors)) {
var c
if(isColorArray) {
if(i < colors.length) {
Expand Down Expand Up @@ -22320,7 +22342,7 @@ proto.update = function(options) {

//Get lineColor
if(!glyphVisible) lineColor = [1,1,1,0]
else if(Array.isArray(lineColors)) {
else if(isArrayOrTypedArray(lineColors)) {
var c
if(isLineColorArray) {
if(i < lineColors.length) {
Expand Down Expand Up @@ -22351,7 +22373,7 @@ proto.update = function(options) {

var size = 0.5
if(!glyphVisible) size = 0.0
else if(Array.isArray(sizes)) {
else if(isArrayOrTypedArray(sizes)) {
if(i < sizes.length) {
size = +sizes[i]
} else {
Expand All @@ -22365,7 +22387,7 @@ proto.update = function(options) {


var angle = 0
if(Array.isArray(angles)) {
if(isArrayOrTypedArray(angles)) {
if(i < angles.length) {
angle = +angles[i]
} else {
Expand All @@ -22390,7 +22412,7 @@ proto.update = function(options) {
var textOffsetY = alignmentY

var textOffsetX = 0
if(Array.isArray(alignmentX)) {
if(isArrayOrTypedArray(alignmentX)) {
if(i < alignmentX.length) {
textOffsetX = alignmentX[i]
} else {
Expand All @@ -22401,7 +22423,7 @@ proto.update = function(options) {
}

var textOffsetY = 0
if(Array.isArray(alignmentY)) {
if(isArrayOrTypedArray(alignmentY)) {
if(i < alignmentY.length) {
textOffsetY = alignmentY[i]
} else {
Expand Down
Loading