-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Embed WaveJSON data in SVG metadata when saving #362
base: trunk
Are you sure you want to change the base?
Conversation
This allows saving or sharing the SVG image while preserving the WaveJSON data. The waveform editor is then able to open SVG files created after this change, and extract the original WaveJSON from the metadata field. A human can also obtain the WaveJSON data by simply opening the SVG file in a text editor.
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.
NOTE: I am not affiliated with this project, but wanted to raise some questions, as the code did not match my expectation based on the issue.
lib/append-save-as-dialog.js
Outdated
@@ -92,6 +93,10 @@ function appendSaveAsDialog (index, output) { | |||
html += firstDiv.innerHTML.substring(166, firstDiv.innerHTML.indexOf('<g id="waves_0">')); | |||
} | |||
html = [div.innerHTML.slice(0, 166), html, div.innerHTML.slice(166)].join(''); | |||
if (obj) { | |||
metadataWaveJSON = '<metadata id="wavejson">' + JSON.stringify(obj, null, 2) + '</metadata>'; |
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.
This should encode the version of wavedrom that generated the output.
This will allow later-discovered bugs to be worked around, for example. Versioning is a Good Thing
™️.
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.
This is a good idea, I'll see if I can retrieve the wavedrom version in this context, and add it to the metadata.
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 it's globally available as WaveDrom.version
(type: string)
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.
After further consideration, I would recommend making this a distinct option in the context menu.
You can expand here for some of the reasons
When adding the exported source JSON, the intent is to enable that text to be imported later. Therefore, have a version field is particularly important (for bugs, versioning, etc.) when including the wavedrom JSON source.
However, when exporting SVG, some folk may prefer to exclude the source. For example, some may edit the SVG manually post-creation, and it would no longer match the exported SVG, so they don't want to have to manually remove outdated metadata. As another example, authors of particularly large or complex diagrams may not want to share the JSON source.
In addition, adding the version number to all SVG output would increase the complexity of regression testing. For example, I'm considering adding some tests that ensure identical output for both PNG and SVG between builds. Adding text in the SVG which should be ignored makes that much more complex.
I think you have a handle on how to add a new option, but let me know if you would like any pointers.
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 have just pushed a new commit which adds the WaveDrom version in the metadata tag, as per your first suggestion.
Personally, I would not overcomplicate it by adding more context menu options to cover cases which will be rarely used. If someone really wants to remove the wavejson data, they can do so very easily with a text editor, otherwise the metadata can simply be ignored.
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.
+1 on removing version with sed (or other tool) if you really want to ...
Does this preserve whitespace in the source? I almost always include extra whitespace in my JSON source in order to get all of the Also, correct me if I'm wrong, but it looks like this PR only implements embedding the metadata on save, correct? It doesn't implement opening/importing an SVG file with the embedded metadata as described:
In my opinion a fully-fleshed out implementation of this idea would also support importing from an SVG file with the embedded metadata/source, ideally in a way where saving as SVG -> importing from SVG is fully lossless (e.g. all whitespace and formatting is preserved). If this was the case, SVG w/ embedded metadata could serve as the main file format for Wavedrom, functioning the same as the JSON format but with the additional benefit that it is easily viewable without the Wavedrom editor. |
Yes! This commit preserves whitespace, it saves the waveJSON exactly as it is input. You're right, this PR does not include the "import" part, it just makes it possible for an editor to do so. I'd love to implement it, but (please correct me if I'm wrong) I think this is not the right repo for that. I've searched a bit and looks like https://github.com/wavedrom/wavedrom.github.io would be the right place for such PR? |
You should be able to verify with inkscape -> Edit -> XML Editor (Shift+Ctrl+X) or just look (with less). When I compare something from the
when that is saved as a svg, the metadata is both formatted differently and some is actually scaled differently.
the
when values are in a string - it's preserved - but if they are not - the values are scaled to their pixel values, not the clock values.... |
This is a right repo for:
https://github.com/wavedrom/wavedrom.github.io -- is a right place to add Load SVG button. |
good stuff! I was poking around about doing the same thing for PNG output. I'll leave some breadcrumbs here for that. |
@drom I would like some clarity on where exactly the 'Export SVG' function is implemented. Is it different for web browser interface vs. nw app ? I primarily work through the browser interface (have an offline copy of wavedrom . github . io contents with the editor.html, and I update the skins / wavedrom.min.js in that folder whenever I make changes in my fork of wavedrom). While trying to port this PR into my fork, I found that the saved SVG file did not have the code embedded as described by other users here. After a bit of debug, I realized that the function that needed modification was in the By the way, @rgetz : if you want to keep the contents of the WaveJS pane as-is in the metadata segment, and you are using the web browser approach described above, this is the replacement I have for the ssvg replacement function
function ssvg () {
let svg, ser, edtr, embedWaveJS, svgString;
svg = document.getElementsByTagName('svg')[0];
embedWaveJS = document.createElementNS("http://www.w3.org/2000/svg", 'metadata');
embedWaveJS.setAttribute("id","WaveJS");
embedWaveJS.setAttribute("version", window.WaveDrom.version);
embedWaveJS.textContent = '\n<!--\n' + WaveDrom.cm.getValue() + '\n-->\n';
svg.appendChild(embedWaveJS);
ser = new XMLSerializer();
svgString = ser.serializeToString(svg);
svg.removeChild(embedWaveJS);
let components = svgString.split(/(<metadata[^>]*>)/);
components[2] = components[2].replaceAll('<','<').replaceAll('>','>').replaceAll('&','&');
return '<?xml version="1.0" standalone="no"?>\n'
+ '<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">\n'
+ '<!-- Created with WaveDrom -->\n'
+ components[0] + '\n' + components[1] + components[2] ;
} I don't claim this to be the most efficient or bug-free code, but it has worked well so far in my limited testing. |
I traced the influence of this PR's modifications to a workflow that is not exercised by users of my custom in-house fork. However, I have tried to make it work in a manner similar to that of the changes I made in editor.js of the wavedrom.github.io repository. There are still some rough edges with saving diagrams with different skins on the same page. I believe they don't even render correctly with the public WaveDrom version even if all relevant skins are sourced under the script tag. So, that does translate to the saved SVGs too, but it is an artifact of WaveDrom's compositing issues reported in #263. First off, here are a couple of sample inputs (in addition to the ones mentioned by @rgetz) for which the current PR delivers unexpected results. Sine and Cosine Waveforms with WaveJS
<script type="WaveDrom">
(function (res) {
let sinTable = [];
let cosTable = [];
for (let i = 0; i < (2*3.1415926*1000); i++) {
sinTable[i] = Math.sin(i/1000) ;
cosTable[i] = Math.cos(i/1000) ;
}
for (let l in res.signal) {
let tlane = res.signal[l] ;
if (typeof tlane.wave === 'object') {
const triglane = (tlane.wave[0] == 'sin') || (tlane.wave[0] == 'cos');
if (triglane) {
let sigtmp = tlane ;
let tphase = tlane.phase || 0 ;
let tspec = tlane.wave ; // wave to mod
tlane.wave = [] ;
let dspec = tspec[1] ;
let nodspec = (typeof dspec === 'undefined') ;
let dspecamp = (nodspec || (typeof dspec.amp === 'undefined')) ? 1 : dspec.amp ;
let dspecf = (nodspec || (typeof dspec.f === 'undefined') || (dspec.f == 0)) ? 1 : dspec.f ;
let dspecphi = (nodspec || (typeof dspec.phi === 'undefined')) ? 0 : dspec.phi ;
let drepeat = (nodspec || (typeof dspec.repeat === 'undefined')) ? 1 : dspec.repeat ;
let pathArr = [] ;
for (let rn = 0; rn < drepeat; rn++) {
for (let i = 0; i < (1/dspecf); i += 0.001) {
let tblIdx = parseInt((2000 * 3.1415926 * i * dspecf) + (dspecphi * 1000 * 3.1415926 * dspecf / 180))
% parseInt(2000 * 3.1415926) ;
let scaledVal = (tspec[0] === "sin") ? sinTable[tblIdx] : (tspec[0] === "cos") ? cosTable[tblIdx] : 0 ;
// scaledVal is between -1 and +1, but pw coords pre-scaling should be between 0 and 1
// first shift the value to be between -0.5 and 0.5 and then apply the amplitude factor
scaledVal /= 2.00 ;
scaledVal *= dspecamp ;
// shift to center around y = 0.5
scaledVal += 0.5 ;
if ((rn == 0) && (i == 0)) {
pathArr.push('M');
}
pathArr.push((rn / dspecf) + i - tphase);
pathArr.push(scaledVal);
}
}
let pwObj = { 'd' : pathArr };
tlane.wave = [ 'pw', pwObj ] ;
res.signal[l] = tlane ;
}
}
}
return res ;
})(
{
config: { skin:'default',hbounds:[0,15.5],hscale:1,marks:true,arcFontSize:8, arcStrokeSize: 1, fit2pane: 1 },
head: {tick:0},
signal: [
{name: 'clkp', wave: 'p..........'}, // Lane 0
{},
{name: "sinB", phase: 0, wave: [ 'sin', { f: 1 , repeat : 3, amp: 1 } ] }, // Lane 2
{},
{name: "sinC", phase: 0, wave: [ 'sin', { f: 0.5 , repeat : 3, amp: 1 } ] }, // Lane 4
{},
{name: 'pll_en', wave: '0.1.........0...'}, // Lane 6
{},
{name: 'pll_out', wave: 'x..<.>'}, // Lane 8 needs 3 overlays - low freq sin, better freq sin, x
{},
{name: 'pll_lock', wave: '0......1.....0..'}, // Lane 10
{},
{name: 'data-digital', wave: 'h.l...h.l.'}, // Lane 12
{},
{name: "PSK-analog", phase: 0, wave: [ 'sin', { f: 1 , repeat : 2, amp: 1 } ] }, // Lane 14 needs 3 overlays
{},
{wave: [ 'tl', { coords: [1,0], text: '' } ], overlayOnLane: 14 }, // Lane 14 needs 3 overlays
{phase: -3.5, wave: [ 'sin', { f: 0.75 , repeat : 2, amp: 1 } ], overlayOnLane: 8}, // pll not locked
{phase: -6.175, wave: [ 'sin', { f: 1.25 , repeat : 9, amp: 1 } ], overlayOnLane: 8}, // pll locked
{phase: -13.375, wave: 'x..', overlayOnLane: 8}, // pll not enabled
{phase: -2, wave: [ 'sin', { f: 1 , repeat : 4, phi: 180 } ], overlayOnLane: 14 }, // data = h
{phase: -6, wave: [ 'sin', { f: 1 , repeat : 2, phi: 0 } ], overlayOnLane: 14 }, // data = l
{phase: -8, wave: [ 'sin', { f: 1 , repeat : 2, phi: 180 } ], overlayOnLane: 14 }, // data = h
],
}
)
</script> Bricks Downscaling Test Waveform
<script type="WaveDrom">
{
config: { skin: 'default', hscale: 0.25, lines: { offset: 2, every: 125 }, fit2pane: true },
head: { tick: -2, every: 10, text: ['tspan', { "font-size": '12px' }, 'based on 4Mhz clock; assumes BE=RDY=1'] },
signal: [
{ name: 'CLK',
wave: '1.0' + '.'.repeat(125-1) + '1' + '.'.repeat(125-1) + '0.', phase: 0.20 },
{ node: '...' + '.'.repeat(105-1) + 'Λ' + '.'.repeat(20-1) + 'T' + '.'.repeat(105-1) + 'Y' + '.'.repeat( 20-1) + 'Ы', phase: .215 },
{ node: '...' + '.'.repeat(100-1) + 'W' + '.'.repeat( 5-1) + 'X' + '.'.repeat( 5-1) + 'A' + '.'.repeat(115-1) + 'Δ'+ '.'.repeat(5-1) + 'O' + '.'.repeat(5-1) + 'Z', phase: .215 },
{ name: 'CLK_SRC',
wave: '0..' + '.'.repeat(100-1)+'x' + '.'.repeat(10-1)+'1' + '.'.repeat(115-1)+'x' + '.'.repeat(10-1)+'0' + '.'.repeat(17-1), phase: 0.20 },
{ node: '..Ύ' + '.'.repeat(30-1) + 'Д', phase: 0.215 },
{ node: '..Б' + '.'.repeat(10-1) + 'Г', phase: 0.215 },
{ name: 'A0..15, RWB',
wave: '3..' + '.'.repeat(10-1)+'x' + '.'.repeat(20-1)+'3' + '.'.repeat(220-1)+'..', data: ['', 'ADDRESS, RWB'], phase: 0.20 },
{ node: '..Ⴄ' + '.'.repeat(44-1)+'Ⴆ', phase: 0.215 },
{ node: '..Ⴀ' + '.'.repeat(10-1)+'<.>Ⴃ', phase: 0.215 },
{ name: 'A16..18',
wave: '7..' + '.'.repeat(10-1) + '<.>x' + '.'.repeat(33-1) + '<.>7' + '.'.repeat(206-1)+'..', data: ['', 'BANK ADDRESS'], phase: 0.20 },
{ node: '..Φ' + '.'.repeat(10-1)+'<.>Έ' + '.'.repeat(114-1)+'<.>Ζ' + '.'.repeat(44-1)+'<.>Η', phase: 0.215 },
{ name: 'WRITE BUFFER',
wave: '6..' + '.'.repeat(10-1)+'<.>x' + '.'.repeat(159-1)+'6' + '.'.repeat(80-1)+'<.5>..', data: ['', 'Writing Data'], phase: 0.20 },
{ node: '..B' + '.'.repeat(69-1)+'<.>Π', phase: 0.215 },
{ node: '..E' + '.'.repeat(11-1)+'<.>F', phase: 0.215 },
{ name: 'RAM_CS',
wave: '0..' + '.'.repeat(11-1)+'<.>x' + '.'.repeat(58-1)+'0' + '.'.repeat(182-1)+'<.>', data: ['', 'ROM'], phase: 0.20 },
{ node: '...' + '.'.repeat(44-1)+'U' + '.'.repeat(55-1)+'V', phase: 0.215 },
{ node: '...' + '.'.repeat(44-1)+'Ο' + '.'.repeat(50-1)+'Ό', phase: 0.215 },
{ node: '...' + '.'.repeat(69-1)+'<.>Β' + '.'.repeat(50-1)+'Ξ', phase: 0.215 },
{ node: '...' + '.'.repeat(10-1)+'P' + '.'.repeat(93-1)+'I' + '.'.repeat(30-1)+'<.>M' + '.'.repeat(45-1)+'N', phase: 0.215 },
{ node: 'Ѳ' + '.'.repeat(12-1)+'Ѵ' + '.'.repeat(90-1)+'Α' + '.'.repeat(3-1)+'Γ' + '.'.repeat(7-1)+'R' + '.'.repeat(23-1)+'<.>S' + '.'.repeat(91-1)+'<.>C' + '.'.repeat(3-1)+
'D' + '.'.repeat(7-1)+'Ψ' + '.'.repeat(17-1)+'Ω', phase: 0.215 },
{ name: 'WR',
wave: 'x..' + '.'.repeat(10-1) + '1' + '.'.repeat(93-1) + 'x' + '.'.repeat(30-1) + '<.>0' + '.'.repeat(94-1) + '<.>x' + '.'.repeat(24-1), data: ['', 'ROM'], phase: 0.20 },
{ node: '...' + '.'.repeat(10-1)+'G' + '.'.repeat(193-1) + 'L' + '.'.repeat(25-1) + 'K', phase: 0.215 },
{ name: 'EXTRAM DATA',
wave: '5..' + '.'.repeat(10-1)+'z' + '.'.repeat(193-1)+'5' + '.'.repeat(49-1), data: ['', 'DATA VALID'], phase: 0.20 },
],
edge: [
'Б+1+Г 10ns', 'Ύ+1+Д 30ns',
'Ⴀ+1+Ⴃ 10.5ns', 'Ⴄ+1+Ⴆ 44ns',
'Φ+1+Έ 10.5ns', 'Ζ+1+Η 44.5ns',
'B+1+Π 69.5ns', 'E+1+F 11.5s',
'Β+1+Ξ 50ns (t<sub>CW</sub>)', 'Ο+1+Ό 50ns (t<sub>AW</sub>)',
'G+1+G 0ns (t<sub>DH</sub>)','L+1+K 25ns (t<sub>DW</sub>)', 'Ч+1+Ш 25ns (t<sub>DW</sub>)',
'I+1+I 0ns (t<sub>AS</sub>)', 'M+1+N 45ns (t<sub>WP</sub>)', 'P+1+P 0ns (t<sub>WR</sub>)',
'U+1+V 55ns (t<sub>WC</sub>)', 'C+1+D 3ns', 'R+1+S 23.5ns', 'Ψ+1+Ω 25ns', 'Α+1+Γ 3ns', 'Ѳ+1+Ѵ 25ns',
'Λ+1+T 20ns', 'W+1+X 5ns', 'X+1+A 5ns', 'Y+1+Ы 20ns', 'Δ+1+O 5ns', 'O+1+Z 5ns',
],
}
</script> In both of the above cases, the embedded metadata is the parsed output / final WaveJSON object that is understood by the WaveDrom engine. However, WaveDrom allows use of normal JavaScript to generate this WaveJSON object - and that is what is done in the above two examples. For the metadata embedding feature to be useful across all use-cases, we should store the WaveJS into the SVG file and not the final WaveJSON object (which could be big and unwieldy / blow up the downloaded SVG size). Anyways, I think the best way to solve this would be the code changes below. The addition of the process-all.js
...
for (let i = 0; i < index; i += 1) {
const obj = eva('InputJSON_' + i);
renderWaveForm(i, obj, 'WaveDrom_Display_', notFirstSignal);
if (obj && obj.signal && !notFirstSignal) {
notFirstSignal = true;
}
appendSaveAsDialog(i, 'WaveDrom_Display_', document.getElementById('InputJSON_' + i).innerHTML);
}
... append-save-as-dialog.js
...
saveSvg.addEventListener('click',
function () {
let svg = (document.getElementById('WaveDrom_Display_' + index).getElementsByTagName('svg'))[0] ;
if (index !== 0) {
const firstSVG = (document.getElementById('WaveDrom_Display_0').getElementsByTagName('svg'))[0] ;
const defs2prepend = (firstSVG.getElementsByTagName('defs'))[0] ;
const style2prepend = (firstSVG.getElementsByTagName('style'))[0] ;
svg.prepend(defs2prepend);
svg.prepend(style2prepend);
}
let embedWaveJS = document.createElementNS('http://www.w3.org/2000/svg', 'metadata');
embedWaveJS.setAttribute('id', 'WaveJS');
embedWaveJS.setAttribute('version', window.WaveDrom.version);
embedWaveJS.textContent = '\n<!--\n' + obj + '\n-->\n';
svg.appendChild(embedWaveJS);
let ser = new XMLSerializer();
let svgString = ser.serializeToString(svg);
svg.removeChild(embedWaveJS);
let components = svgString.split(/(<metadata[^>]*>)/);
components[2] = components[2].replaceAll('<', '<').replaceAll('>', '>').replaceAll('&', '&');
const svg2dl = '<?xml version="1.0" standalone="no"?>\n'
+ '<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">\n'
+ '<!-- Created with WaveDrom -->\n'
+ components[0] + '\n' + components[1] + components[2] ;
const a = document.createElement('a');
a.href = 'data:image/svg+xml;base64,' + btoa(unescape(encodeURIComponent(svg2dl)));
a.download = 'wavedrom.svg';
a.click();
menu.parentNode.removeChild(menu);
document.body.removeEventListener('mousedown', closeMenu, false);
},
false
);
... Anyone with a code base closer to the current master can try out these changes and submit a PR if satisfied. |
I got down to processing one of the metadata-embedded SVGs through the W3 SVG validator to ensure that #323 continued to remain in a fixed state in my fork. I learned that metadata entries in SVG 1.1 can't have random attributes. As a result, the 'version' entry has to be removed. I am planning to update the metadata section contents with the string '<!--\n// WaveJS processed with WaveDrom version 3.2.0\n' instead. This will still allow us to grab the generating version in the 'Load SVG' case. |
This allows saving, embedding, or sharing the SVG image while preserving the WaveJSON data.
The waveform editor is then able to open SVG files created after this change, and extract the original WaveJSON from the metadata field with id "wavejson". A human can also obtain the WaveJSON data by simply opening the SVG file in a text editor.
Issue #361