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.

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

Add option to adjust the viewbox to match the width of the svg #43

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from
Open
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion index.html
Original file line number Diff line number Diff line change
@@ -46,9 +46,10 @@ <h1>Skiafy</h1>
<br/>
<span class="limitation">Works only with fill attributes & expects colors in form #FFF or #FFFFFF.</span>
</label>
<label><input id="fix-viewbox" type="checkbox"> Translate and scale the viewbox to match the width of the svg</label><br/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I enable this checkbox by default? We usually don't get the right result if the width/height of the svg doesn't match the viewbox. If it matches, this checkbox is a no-op

Copy link
Owner

Choose a reason for hiding this comment

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

Only thing I'm worried about is how this breaks in cases we haven't considered like a non-square size, or unspecified size, or non-square viewbox. Therefore it might be safer to not check it by default.

Also can we update the text to

"""
Normalize viewbox and path coordinates to match the specified width of the SVG (assuming square dimensions).
"""

</p>
<p>
Translate (applies after flip):<br/>
Translate (applies after flip and scale):<br/>
<input id="transform-x" placeholder="x"><br/>
<input id="transform-y" placeholder="y">
</p>
4 changes: 3 additions & 1 deletion main.js
Original file line number Diff line number Diff line change
@@ -23,16 +23,18 @@ function ConvertInput() {
var scaleX = $('flip-x').checked ? -1 : 1;
var scaleY = $('flip-y').checked ? -1 : 1;
var preserveFill = $('preserve-fill').checked;
var fixViewbox = $('fix-viewbox').checked;

var input = $('user-input').value;
$('svg-anchor').innerHTML = input;
var output = '';
var svgNode = $('svg-anchor').querySelector('svg');

try {
output = ProcessSvg(svgNode, scaleX, scaleY, translateX, translateY, preserveFill);
output = ProcessSvg(svgNode, scaleX, scaleY, translateX, translateY, preserveFill, fixViewbox);
} catch (e) {
$('output-span').textContent = e.name + ": " + e.message;
console.log(e);
return;
}

5 changes: 4 additions & 1 deletion run_tests.js
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ testCases = [
{ name: 'color_home', preserveFill: false, expected: 'home' },
{ name: 'implicit_lineto' },
{ name: 'implicit_r_lineto' },
{ name: 'fix_viewbox', fixViewbox: true },
]

function runTests() {
@@ -29,8 +30,10 @@ function runTests() {
svgAnchor.innerHTML = testData[name].svg;

var preserveFill = testCases[i].preserveFill || false;

var fixViewbox = testCases[i].fixViewbox || false;

var output = ProcessSvg(svgAnchor.querySelector('svg'), 1, 1, 0, 0, preserveFill);
var output = ProcessSvg(svgAnchor.querySelector('svg'), 1, 1, 0, 0, preserveFill, fixViewbox);
Copy link
Owner

Choose a reason for hiding this comment

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

nit: maybe rather than fixViewbox everywhere it should be normalizeCoordinates? You're changing all the coordinates in the paths, not just the viewbox.


var expected = testCases[i].expected || name;
var expectedOutput = testData[expected]['expected'];
18 changes: 16 additions & 2 deletions skiafy.js
Original file line number Diff line number Diff line change
@@ -323,14 +323,28 @@ function HandleNode(svgNode, scaleX, scaleY, translateX, translateY, preserveFil
return output;
}

function ProcessSvg(svgNode, scaleX, scaleY, translateX, translateY, preserveFill) {
function ProcessSvg(svgNode, scaleX, scaleY, translateX, translateY, preserveFill, fixViewbox) {
var output = '';
var canvasSize = svgNode.viewBox.baseVal.width;
if (canvasSize == 0)
canvasSize = svgNode.width.baseVal.value;

if (fixViewbox) {
Copy link
Owner

Choose a reason for hiding this comment

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

nit: document what's happening/what this is for

var scaleFactor = svgNode.width.baseVal.value / svgNode.viewBox.baseVal.width;
Copy link
Owner

Choose a reason for hiding this comment

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

what if there is no width specified?

canvasSize *= scaleFactor;
scaleX *= scaleFactor;
scaleY *= scaleFactor;
if (svgNode.viewBox.baseVal.y < 0) {
translateY -= svgNode.viewBox.baseVal.y * scaleFactor;
}
if (svgNode.viewBox.baseVal.x < 0) {
translateX -= svgNode.viewBox.baseVal.x * scaleFactor;
}
}

if (canvasSize != 48)
output += 'CANVAS_DIMENSIONS, ' + canvasSize + ',\n';

Copy link
Owner

Choose a reason for hiding this comment

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

nit: remove spaces

output += HandleNode(svgNode, scaleX, scaleY, translateX, translateY, preserveFill);
// Truncate final comma and newline.
return output.slice(0, -2);
15 changes: 15 additions & 0 deletions test_data.js
Original file line number Diff line number Diff line change
@@ -74,4 +74,19 @@ R_MOVE_TO, 1, 2,
R_LINE_TO, 4, 3,
R_LINE_TO, 1, 1,
CLOSE`},

'fix_viewbox': {
svg:
`
<svg xmlns="http://www.w3.org/2000/svg" height="48" viewBox="0 -960 960 960" width="48"><path d="M378-246 154-470l43-43 181 181 384-384 43 43-427 427Z"/></svg>`,
expected:
`MOVE_TO, 18.9f, 35.7f,
LINE_TO, 7.7f, 24.5f,
R_LINE_TO, 2.15f, -2.15f,
R_LINE_TO, 9.05f, 9.05f,
R_LINE_TO, 19.2f, -19.2f,
R_LINE_TO, 2.15f, 2.15f,
R_LINE_TO, -21.35f, 21.35f,
CLOSE`},

};