wavedrom icon indicating copy to clipboard operation
wavedrom copied to clipboard

Purpose of notFirstSignal in Code Base

Open Ganesh-AT opened this issue 1 year ago • 1 comments

Aliaksei @drom ,

I have been primarily using WaveDrom through the WaveDromEditor browser interface. I am quite familiar with major portions of the code base for both WaveDrom and wavedrom.github.io, but I realized yesterday that there are some segments that are utilized in workflows that I am not used to. While working on a proper fix for the PR submitted in #362, I found something that I would like more clarity on.

I see a notFirstSignal boolean variable that is passed from ProcessAll in process-all.js all the way down to insertSVGTemplate in insert-svg-template.js. Its sole purpose seems to be the avoid repeating the style and defs sections for the second SVG onwards (when there are multiple WaveDrom type scripts on the same page). Multiple SVGs are being rendered in distinct divs with different IDs. In that case, can you shed light on why the style and defs are not being embedded into each SVG? I am planning to do away with that variable in my fork (but I still want my fork to be compliant with anything accepted by WaveDrom). Is there any other use for notFirstSignal in some other workflow that I am not considering here?

As I mentioned in my comment in #362, WaveDrom doesn't render multiple SVGs with different skins on the same page correctly. I reported this back in 2019 in #263, albeit for a different use-case (editing outputs in Inkscape). Over the years, I have made many changes in my fork, and one of them was mass-renaming of the id of all elements in all skins to follow the pattern : 'wd_<skin-name>_<original-id>' (that was also instrumental in validating all SVG outputs as compliant - resolving #323). One of the advantages of doing this was that I could have multiple skins in the same diagram.

Prior to yesterday, I had never actively tried the 'multiple WaveDrom WaveJS scripts in a single HTML page' input. In preparing a solution for #362, I am testing out multiple multi-skin WaveJS inputs in a single page. The basic test case is the first SVG with default skin, and the second one with narrow skin. After disabling the notFirstSignal feature, I was able to get the style and defs in each SVG separately, but the lane parameters for the narrow waveform was from the default one. After commenting out the logic based on the index parameter in the laneParamsFromSkin function of render-signal.js, I was able to get the rendering of all diagrams on the page correctly.

Do you foresee any issues for workflows that I have not considered due to the following changes?

process-all.js (Line 24 onwards)


    let notFirstSignal = false;
    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);
    }

render-signal.js (Line 13 onwards)


function laneParamsFromSkin (index, source, lane, waveSkin) {

    //if (index !== 0) {
    //    return;
    //}

    const waveSkinNames = Object.keys(waveSkin);

Ganesh-AT avatar Nov 27 '23 04:11 Ganesh-AT

@Ganesh-AT , you mention you have a forked version of this where you have fixed all the SVG issues and they pass, since it seems there's a real lack of progress on the main branch for these fixes, how can I get ahold of your fork?

wbrisett avatar Jun 27 '24 09:06 wbrisett