Skip to content

Commit

Permalink
Safari SVG Text workaround, see #1616
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathanolson committed Feb 29, 2024
1 parent 3af2877 commit df0a527
Showing 1 changed file with 44 additions and 3 deletions.
47 changes: 44 additions & 3 deletions js/display/drawables/TextSVGDrawable.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ define( function( require ) {
// See https://github.com/phetsims/scenery/issues/455 for more information.
var useSVGTextLengthAdjustments = !platform.ie && !platform.edge;

// Safari seems to have many issues with text and repaint regions, resulting in artifacts showing up when not correctly
// repainted (https://github.com/phetsims/qa/issues/1039#issuecomment-1949196606), and
// cutting off some portions of the text (https://github.com/phetsims/scenery/issues/1610).
// We have persistently created "transparent" rectangles to force repaints (requiring the client to do so), but this
// seems to not work in many cases, and seems to be a usability issue to have to add workarounds.
// If we place it in the same SVG group as the text, we'll get the same transform, but it seems to provide a consistent
// workaround.
var useTransparentSVGTextWorkaround = platform.safari;

/**
* A generated SVGSelfDrawable whose purpose will be drawing our Text. One of these drawables will be created
* for each displayed instance of a Text node.
Expand All @@ -37,10 +46,29 @@ define( function( require ) {
this.initializeSVGSelfDrawable( renderer, instance, true, keepSVGTextElements ); // usesPaint: true

if ( !this.svgElement ) {
// @protected {SVGTextElement} - Sole SVG element for this drawable, implementing API for SVGSelfDrawable
var text = this.svgElement = document.createElementNS( scenery.svgns, 'text' );
var text = document.createElementNS( scenery.svgns, 'text' );
text.appendChild( document.createTextNode( '' ) );

// @private {SVGTextElement}
this.text = text;

// If we're applying the workaround, we'll nest everything under a group element
if ( useTransparentSVGTextWorkaround ) {
var group = document.createElementNS( scenery.svgns, 'g' );
group.appendChild( text );

this.svgElement = group;

// "transparent" fill seems to trick Safari into repainting the region correctly.
this.workaroundRect = document.createElementNS( scenery.svgns, 'rect' );
this.workaroundRect.setAttribute( 'fill', 'transparent' );
group.appendChild( this.workaroundRect );
}
else {
// @protected {SVGTextElement|SVGGroup} - Sole SVG element for this drawable, implementing API for SVGSelfDrawable
this.svgElement = text;
}

// TODO: flag adjustment for SVG qualities
text.setAttribute( 'dominant-baseline', 'alphabetic' ); // to match Canvas right now
text.setAttribute( 'text-rendering', 'geometricPrecision' );
Expand All @@ -62,7 +90,7 @@ define( function( require ) {
* Implements the interface for SVGSelfDrawable (and is called from the SVGSelfDrawable's update).
*/
updateSVGSelf: function() {
var text = this.svgElement;
var text = this.text;

// set all of the font attributes, since we can't use the combined one
if ( this.dirtyFont ) {
Expand All @@ -81,6 +109,19 @@ define( function( require ) {
// text length correction, tested with scenery/tests/text-quality-test.html to determine how to match Canvas/SVG rendering (and overall length)
if ( this.dirtyBounds && useSVGTextLengthAdjustments && isFinite( this.node.selfBounds.width ) ) {
text.setAttribute( 'textLength', this.node.selfBounds.width );

if ( useTransparentSVGTextWorkaround ) {
// Since text can get bigger/smaller, lets make the region larger than the "reported" bounds - this is needed
// for the usually-problematic locales that have glyphs that extend well past the normal browser-reported
// bounds. Since this is transparent, we can make it larger than the actual bounds.
var paddingRatio = 0.2;
var horizontalPadding = this.node.selfBounds.width * paddingRatio;
var verticalPadding = this.node.selfBounds.height * paddingRatio;
this.workaroundRect.setAttribute( 'x', this.node.selfBounds.minX - horizontalPadding );
this.workaroundRect.setAttribute( 'y', this.node.selfBounds.minY - verticalPadding );
this.workaroundRect.setAttribute( 'width', this.node.selfBounds.width + 2 * horizontalPadding );
this.workaroundRect.setAttribute( 'height', this.node.selfBounds.height + 2 * verticalPadding );
}
}

// Apply any fill/stroke changes to our element.
Expand Down

0 comments on commit df0a527

Please sign in to comment.