Dav1dde / gl3n

OpenGL Maths for D (not glm for D).
http://dav1dde.github.com/gl3n/
Other
103 stars 49 forks source link

separated linalg module into a package #67

Open extrawurst opened 8 years ago

extrawurst commented 8 years ago

i have simply split up the module, no other changes for now. 1) i think the linalg module was huge 2) it is transparent since i created a package.d for the new package 3) this makes it possible to implement alternative matrix-memory layouts without impacting the other types (see #66)

what do you think ?

Dav1dde commented 8 years ago

1) It was relatively big, but not too big that it required a split imo. 2) I wonder if there are corner cases, where it doesn't work (as basically everywhere in D) 3) In what way would that help?

extrawurst commented 8 years ago

1) 2,5k file size with three totally different objects, at least violating single-responsibility rule on module basis is too big in my opinion. and mono-d had a considerable hickup on my machine opening that file.

2) well at least the unittest do run on my end. also my projects all run fine using the new structure. even if there is a corner case that breaks there is the issue board for that iff we agree that this structure is desirable.

3) I was thinking about implementing the other layout in another source file and import in the package depending on the specified (or default) version provided. and if not that there is still the limited focus since these layout topics just concern the matrix object.

what is you objection besides the possibility of introducing bugs, even though I did not change any code?

Dav1dde commented 8 years ago

1) Well, I would blame Mono-D for that. 2) I don't think it should break any code, was just interested if there maybe are some known bugs. 3) Mh, I see, but I don't want to provide a matrix implementation for row major and column major. It's basically more or less useless code duplication. The decision I made when I started gl3n was maybe bad, but it is too late to change now.

I am not against it, just considering options. E.g. this change would break backwards compatibility e.g., this change breaks with older DMD versions. But I don't see a general advantage (except maintainability).

DrInfiniteExplorer commented 7 years ago

There are no obvious drawbacks in accepting this pull request, right? Proper separation is a good thing. The discussion about column/row is a separate issue, and to not impact the decision to go forward with this PR.

Dav1dde commented 7 years ago

I still don't see a benefit in splitting it (yet). If there will be a split in the future it will also involve changing more files, I really don't like that ext module etc.

extrawurst commented 7 years ago

what do u mean by "ext module"?

Dav1dde commented 7 years ago

gl3n.ext

DrInfiniteExplorer commented 7 years ago

Ah, that gl3n.ext entered the discussion pretty randomly. Thought it was something related to these changes, was wondering why I couldn't find it in the changes for this PR.

Anyway.. what is the plan/vision then? If you can make a description of what your plan is going forward, maybe we can help with progressing that plan

Fr3nchK1ss commented 3 years ago

This change would be great. From my personal XP:

I can do it, just need the green light. Also DServer under VisualSC gives tons of warnings, it would be nice to have clean cut modules to work those warnings incrementally.