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

Issue with proportional spacing #13

Open
jdotpo opened this issue Jun 25, 2023 · 4 comments
Open

Issue with proportional spacing #13

jdotpo opened this issue Jun 25, 2023 · 4 comments

Comments

@jdotpo
Copy link

jdotpo commented Jun 25, 2023

Thanks for the addition of this functionality, hopefully I haven't misunderstood something...

Currently in fretfind.html, working_area is calculated by taking the "string width at the nut" value and subtracting the sum of all the strings. Since "string width at the nut" is from center-to-center between the first and last string, I believe working_area needs to account for the half of each string outside of that distance.

Also, I'm probably missing something on why the offset calculation is using the total length and "string width at the nut"... couldn't the next_space just be used as is?

I updated the following method locally and it seems to match my spreadsheet calculations:

var computeOffsets = function(strings, gauges, actual_length, perp_width, spacingMode) {
    var offsets = [0];
    const corrected_gauges = spacingMode === 'proportional' ? gauges : new Array(strings).fill(0);
    // If proportional, don't count the 1/2 of the first and last string outside of the working_area
    const propOffset = spacingMode === 'proportional' ? ((gauges[0] + gauges[strings-1])/2) : 0;
    const working_area = perp_width - (corrected_gauges.reduce((tot, cur) => tot + cur)) + propOffset;
    const perp_gap = working_area / (strings - 1);
    for (var i=1; i<strings-1; i++) {
        half_adjacent_strings = (corrected_gauges[i-1] + corrected_gauges[i]) / 2.0;
        next_space = perp_gap + half_adjacent_strings; 
        offsets.push(offsets[i-1] + next_space);
    }
    return offsets;
};

I haven't contributed to a public repo before, but if the above makes sense and you'd like me to get a branch prepared, I can do that. Thanks!

@jdotpo
Copy link
Author

jdotpo commented Jun 25, 2023

The above doesn't work for multiscale... I'll do some more testing around the offset calculation.

@jdotpo
Copy link
Author

jdotpo commented Jun 25, 2023

Ok, at least I understand the original offset calculation now... I had to account for the scale length selection, so I updated the method and the lines that called it:

var computeOffsets = function(strings, gauges, actual_length, perp_width, spacingMode, lengthMode) {
    var offsets = [0];
    const corrected_gauges = spacingMode === 'proportional' ? gauges : new Array(strings).fill(0);
    // If proportional, don't count the 1/2 of the first and last string outside of the working_area
    const propOffset = spacingMode === 'proportional' ? ((gauges[0] + gauges[strings-1])/2) : 0;
    const working_area = perp_width - (corrected_gauges.reduce((tot, cur) => tot + cur)) + propOffset;
    const perp_gap = working_area / (strings - 1);
    for (var i=1; i<strings-1; i++) {
        half_adjacent_strings = (corrected_gauges[i-1] + corrected_gauges[i]) / 2.0;
        next_space = perp_gap + half_adjacent_strings; 
        if(lengthMode === 'single') {
            offsets.push(offsets[i-1] + next_space);
        } else {
            offsets.push(offsets[i-1] + next_space * actual_length / perp_width);
        }
    }
    return offsets;
};

@acspike
Copy link
Owner

acspike commented Jun 25, 2023

Thanks for taking this on. It seems like you are on the right track to me, but I'd really like to know what @gmossessian thinks since it relates more closely to his recent additions than to my original work.

There are some angled parking spaces along my street that are terrible to park. The are much too thin. The spacing between lines at the curb is what you would expect for a normal parking spot. But the distance along the perpendicular from one line to the other is much less because the lines are angled. I wonder if we need to account for the angle of the strings somewhere in this calculation.

@jdotpo
Copy link
Author

jdotpo commented Jun 28, 2023

The existing calculations already account for the differences in distances between a perpendicular and angled nut, so that should be all set. I cleaned up my unnecessary lengthMode logic (as seen above) and put it in a branch in my forked repo... hopefully it's easier to understand my changes now. Thanks!

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

2 participants