asakeron / cljs-webgl

WebGL binding to ClojureScript
109 stars 14 forks source link

Break up constants into section specific namespaces #5

Open rm-hull opened 10 years ago

rm-hull commented 10 years ago

Was wondering whether it might be an idea make better use of namespaces to separate the different groups of constants (almost treating them as pseudo-enums): this might work by creating a src/cljs-webgl/constants folder, and then break up all the constants into smaller files, eg. src/cljs-webgl/constants/begin-mode.cljs would contain:

;; BeginMode
(ns cljs-webgl.constants.begin-mode)

(def points 0x0000)
(def lines 0x0001)
(def line-loop 0x0002)
(def line-strip 0x0003)
(def triangles 0x0004)
(def triangle-strip 0x0005)
(def triangle-fan 0x0006)

src/cljs-webgl/constants/blending-factor-dest.cljs would contain:

;; BlendingFactorDest
(ns cljs-webgl.constants.blending-factor-dest)

(def zero 0)
(def one 1)
(def src-color 0x0300)
(def one-minus-src-color 0x0301)
(def src-alpha 0x0302)
(def one-minus-src-alpha 0x0303)
(def dst-alpha 0x0304)
(def one-minus-dst-alpha 0x0305)

etc.

Then referencing:

(ns blah
  (:require
    [cljs-webgl.constants.begin-mode :as begin-mode]
    ...))

...

(draw! 
  gl-context
  :shader shader-program
  :draw-mode begin-mode/triangles
  ...)
asakeron commented 10 years ago

Yes, that sounds like a good ideia.

I've been thinking about replacing the constants with keywords for some time now as it would require some sort of lookup function and this could work as a way of self documenting the accepted values.

I believe your idea would suffice, though. What do you think?

rm-hull commented 10 years ago

I did wonder about keywords, but there will be a cost associated with the lookup function - whereas this'll come for free using namespaces and defs (I'm presuming) as the constants will get replaced at compile time. The only downside of using namespaces is that there will be a proliferation of source files (I dont think you can have many namespaces in one file?)

asakeron commented 10 years ago

I don't see a larger number of files as a big problem, and the fact that each of them defines related constants is a bonus for readability. So yes, lets do it! :smiley:

asakeron commented 10 years ago

Fixed in 658570d06ca0444bbaca3b84f8b77edb11e5eae6.

rm-hull commented 10 years ago

Nice one :+1:

I fixed a typo - s/texure/texture/ in one of the cljs file

Also, I wonder if namespaces texture-mag-filter and texture-min-filter should be collapsed into a single namespace texture-filter ? what do you think?

Reason I ask is I've currently got a call out to (not checked this in yet):

(init-texture gl
  :image img
  :texture tex2
  :parameters {texture-parameter-name/texture-mag-filter texture-mag-filter/linear
               texture-parameter-name/texture-min-filter texture-mag-filter/linear})

seems a bit odd to have a key texture-parameter-name/texture-min-filter with value texture-mag-filter/linear

Collapsing to a single namespace this becomes:

(init-texture gl
  :image img
  :texture tex2
  :parameters {texture-parameter-name/texture-mag-filter texture-filter/linear
               texture-parameter-name/texture-min-filter texture-filter/linear})

Other option might be to replicate linear & nearest:

(ns cljs-webgl.constants.texture-min-filter)

(def nearest 0x2600)
(def linear  0x2601)

(def nearest-mipmap-nearest 0x2700)
(def linear-mipmap-nearest  0x2701)
(def nearest-mipmap-linear  0x2702)
(def linear-mipmap-linear   0x2703)

and then this looks like:

(init-texture gl
  :image img
  :texture tex2
  :parameters {texture-parameter-name/texture-mag-filter texture-mag-filter/linear
               texture-parameter-name/texture-min-filter texture-min-filter/linear})
asakeron commented 10 years ago

@rm-hull I merged texture-min-filter and texture-mag-filter into a single namespace called texture-filter in 0cbd38ec0b63f8c3e3d427c83708ef35e60a06a5 as you suggested. There are probably other namespaces that can be improved, so I will keep this issue open.

PS: I used the list of values in the WebGL Specification as a reference to break the constants into namespaces.