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

Purpose of notFirstSignal in Code Base #400

Open
Ganesh-AT opened this issue Nov 27, 2023 · 0 comments
Open

Purpose of notFirstSignal in Code Base #400

Ganesh-AT opened this issue Nov 27, 2023 · 0 comments

Comments

@Ganesh-AT
Copy link

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant