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

BSpline::calculate/derivative (and their calling functions) are silently failing for degre != 3 #6313

Closed
omicron321 opened this issue Dec 15, 2020 · 4 comments

Comments

@omicron321
Copy link

/** Calculates the n-degree b-spline value for the given span (i) at the given position (u).
* @param out The Vector to set to the result.
* @param i The span (0<=i<spanCount) spanCount = continuous ? points.length : points.length - degree
* @param u The position (0<=u<=1) on the span
* @param points The control points
* @param degree The degree of the b-spline
* @param continuous If true the b-spline restarts at 0 when reaching 1
* @param tmp A temporary vector used for the calculation
* @return The value of out */
public static <T extends Vector<T>> T calculate (final T out, final int i, final float u, final T[] points, final int degree,
final boolean continuous, final T tmp) {
switch (degree) {
case 3:
return cubic(out, i, u, points, continuous, tmp);
}
return out;
}
/** Calculates the n-degree b-spline derivative for the given span (i) at the given position (u).
* @param out The Vector to set to the result.
* @param i The span (0<=i<spanCount) spanCount = continuous ? points.length : points.length - degree
* @param u The position (0<=u<=1) on the span
* @param points The control points
* @param degree The degree of the b-spline
* @param continuous If true the b-spline restarts at 0 when reaching 1
* @param tmp A temporary vector used for the calculation
* @return The value of out */
public static <T extends Vector<T>> T derivative (final T out, final int i, final float u, final T[] points, final int degree,
final boolean continuous, final T tmp) {
switch (degree) {
case 3:
return cubic_derivative(out, i, u, points, continuous, tmp);
}
return out;
}

@NathanSweet
Copy link
Member

I assume having a degree parameter that only supports 3 is because the author didn't have time to implement others. Contributions are welcome.

@tommyettinger
Copy link
Member

I think the problem is more that the spline doesn't do anything when degree is not 3 (hence "silently" failing), and it isn't documented that 3 is the only spline degree that does anything. Is the solution here for someone to implement other degrees? Would it be enough to document that degree should always be 3?

@NathanSweet
Copy link
Member

NathanSweet commented Dec 19, 2020

Aye, we should document and throw illegal argument exception if it's not 3 until the time that someone implements other values.

Edit: oops, made that change in the texture atlas improvement PR. Will just wait for that to be merged.

NathanSweet added a commit that referenced this issue Dec 20, 2020
* Changed TextureAtlas parsing to store unrecognized values as name/value pairs. Moved split and pad to name/value pairs. Changed TexturePacker to omit entries that are default values and write a pma entry for each page.

* Default original size to packed size. Comment field map sizes.

* Javadoc typo.

* Spaces before commas.

* Added bounds/offsets entries to reduce the number of entries needed per region.

* Use equalsIgnoreCase when comparing entry values.

* Check the index first, which is cheaper.

* Skip sorting regions when there are no indexes.

* Added CHANGES entry for #6316.

* Added a pretty print texture packer setting, enabled by default.

* Added a legacy output setting.

* Mentioned the legacyOutput setting in CHANGES.

* Filter used by issue_pack wasn't GWT-compatible.

* Fixed mipmaps not being created when the min filter needs them (fixes AtlasIssueTest).

* Default legacyOutput to true.

* Added optional header entries.

* Set default for Page#format field.

* Ignore values after the 4th.

* Ignore non-int values for unknown region entries.

* Revert change to issue_pack, since the issue was fixed.

And fixed in a more thorough way, too.

* CHANGES entry for AtlasRegion breaking change.

* CHANGES for Region too.

* BSpline#calculate, throw if degree is not 3, javadoc.

#6313

* Clean up. Set the texture on the page rather than using a map.

* Move TextureAtlasData member class to end of TextureAtlas.

Drives my OCD nuts to put member classes at the top.

* Bunch of clean up.

* Reuse region names/values arrays.
* Initialize index to -1 by default.
* Comments to note deprecated fields.
* Use @null.
* 1024 BufferedReader size.
* Don't need to store index comparator on a field.
* Changed equalsIgnoreCase to equals. Most of the format is case sensitive so seemed odd to ignore case only in some places.
* Use ensureCapacity for textures and regions collections.
* Clean up wonky texture initialization.
* Don't need a static field for reading the entry, plus it wasn't thread safe.
@tommyettinger
Copy link
Member

Now that the texture atlas PR is merged, the requirement for spline degree to be 3 is documented and the method throws an IllegalArgumentException if degree is not 3. If anyone intends to implement more degrees later, we could leave this open until those are ready, but otherwise this could maybe be closed.

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

No branches or pull requests

4 participants