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

func ClipPlane(plane GLenum, equation *float64) #154

Open
mki1967 opened this issue Jun 10, 2014 · 6 comments
Open

func ClipPlane(plane GLenum, equation *float64) #154

mki1967 opened this issue Jun 10, 2014 · 6 comments

Comments

@mki1967
Copy link

mki1967 commented Jun 10, 2014

Shouldn't the equation be a pointer to an array of four doubles?

@dmitshur
Copy link
Member

Yeah, I think it should, similarly to gl.Color4dv.

@tildeleb
Copy link
Contributor

Sorry I duped #157.
Here are the extended issues:

  1. Simple fix or more complicated discussion?
    Should I set up a pull request just to fix ClipPlane()? That was my immediate need and I've already fixed that one. It is completely broken. I don't believe there are any other functions broken in the same way in "gl.go" but someone should confirm and I didn't check the other files.
  2. Consistency and compatibility
    Should we break API compatibility and change other functions such as EvalCoord1fv, EvalCoord2fv, Normal3dv, and friends to use slices instead of fixed size arrays?
    If not how to we explain to new users the logic behind that? If not now, when?
    As someone pointed out in Bad equation argument for gl.ClipPlane #157 "The slice is definitely better than an array. The reason is that it's trivial to go from an array to a slice (array[:]) but difficult to go from a slice to an array (requires unsafe or a copy). The only question is whether we want to bounds check the slice, I'd prefer not, but it's not a huge deal either way."
    Said another way slices are one of Go's main features and what most people use instead of arrays. They are more flexible because they allow you to take an existing slice or array and pass the section you want.
  3. Should we length check slices?
    The big issue with slices is the bounds checking. With the static array types you are guaranteed to get the correct number of elements of the base type. With slices you aren't and garbage could be passed to OpenGL. Effectively a static type check becomes an optional dynamic type check with slices. Unlike the previous caller I believe it would be a good idea to check the size of the slice.
  4. In general some of these decisions like slice or array need to be decided once and they put into a philosophy or design goals and non-goals document. See the next point.
  5. Performance
    Adding a length check, which is really a dynamic type check with affect performance somewhat. OTOH using cgo is already a (large) performance hit. I saw some benchmarks recently and was surprised and the amount of overhead. If someone is going to complain about the performance of the length check in this shim like API then cgi needs to be the first topic of discussion. For the work I am doing so far it's not an issue but if I was writing a high performance game I am not so sure I could live with performance of cgo. This is not a dig against go-gl, just a fact of life with cgo.

@pwaller
Copy link
Contributor

pwaller commented Jun 30, 2014

Phew, lots of things to discuss here. Struggling to summon the brain cycles for it all. So sorry if I don't reach it all before I run out of time. I am but one maintainer on this project and so don't consider me authoritative, it's just my 2c.

  1. I'm in favour of a pull request to fix ClipPlane if it is broken and inconsistent. It sucks if we break code in the wild but in this case if existing code depending on this is buggy or problematic then we should fix it.

  2. Deserves an issue for discussion in its own right and I won't cover it here, except to reference API breaking changes to accept arrays of fixed length #76 Public API changes. #77. I have a vague recollection that in that era, arrays seemed like the best thing at the time. Unfortunately my memory isn't awesome and I don't have tons of time so can't dig very deep. I can't remember the full reasoning. It was thought that it was better to do the type checking at static-time where possible and force people to use arrays. In hindsight I'm not sure it was the right decision. However, I'm not a huge fan of breaking backwards compatibility unless we really must. Which maybe we should, but I'd like to see a very compelling reason.

  3. I would like bounds checking. I don't mind if at run or compile time. I have a tendency to prefer compile time but if it breaks things badly then I would reconsider.

  4. Good idea, don't have the spare cycles myself. If someone else wanted to lead the effort I would chip in (as in this response).

  5. I would assume length checks to be free in the grand scheme of things. If someone cares strongly enough they can make a fork without them. In the meantime they give early warning of lots of easy to make problems. If there are specific circumstances with data which show it to be bad for specific functions we can reconsider.

@tildeleb
Copy link
Contributor

tildeleb commented Jul 1, 2014

I submitted the pull request for ClipPlane.

@tildeleb
Copy link
Contributor

tildeleb commented Jul 1, 2014

I'll take some time to read some of the previous issues and look at older commits to see if I can form a considered opinion about the slice vs array issue. From what I can see from briefly reading #76 and #77 someone felt strongly that arrays were better because of the static type checking and the API was already switched from slices to arrays. It doesn't make sense to keep switching back and forth between the two styles.

@pwaller
Copy link
Contributor

pwaller commented Jul 1, 2014

👍

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

4 participants