oakes / play-clj

A Clojure game library
The Unlicense
939 stars 73 forks source link

[request] add support for the (pixmap) and (width height format) constructors for pixmaps. #51

Closed joshuafcole closed 10 years ago

joshuafcole commented 10 years ago

It's not insurmountable, but it's unpleasant to have to dive into native to create a pixmap from scratch, and it's really unpleasant to have to call operations one at a time with pixmap! since you can't feed an existing one in. How do you feel about adding support for this to bring pixmaps up to par with textures? If you think it's generally a good idea but don't have the free time, I can take a stab at it. Thanks!

oakes commented 10 years ago

Sounds good; how about we add another arity to pixmap*? We can't really work it into the pixmap macro in the current design, but it would be easy to support (pixmap* width height format).

joshuafcole commented 10 years ago

I was actually just taking a crack at it and realized the same thing. I think that makes a lot of sense. My implementation currently looks like:

(defn pixmap*
  ([arg]
    (cond
     (isa? (type arg) Pixmap) arg
     :else
     (let [^FileHandle fh (if (string? arg)
                            (files! :internal arg)
                            arg)]
       (or (u/load-asset (.path fh) Pixmap)
           (Pixmap. fh)))))
  ([width height fmt]
   (Pixmap. width height fmt)))

I think adding a pixmap-format function (akin to the shape-type) function for the formats would also be nice.

In addition (newb question time!) Do you have any examples of pixmaps in action in play-clj? The render loop is a little opaque to me (it looks like things self-register as entities into a global system... sometimes?) and while I can get e.g. labels to render, I'm having trouble getting pixmaps to. I've done a bit of hackery with libgdx interop for shape rendering which worked well enough, and tried to apply that here, but there doesn't seem to be a direct analog for sprites in play-clj that I can access (since usually I've seen pixmaps used as pixmap > manipulation > texture > sprite

render), so I'm a bit stuck.

Any advice would be much appreciated. Otherwise, thanks for the speedy response and I'm looking forward to seeing the more flexible pixmap* function.

Cheers

On Fri, Sep 26, 2014 at 8:17 PM, Zach Oakes notifications@github.com wrote:

Sounds good; how about we add another arity to pixmap? We can't really work it into the pixmap macro in the current design, but it would be easy to support (pixmap width height format).

— Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/issues/51#issuecomment-57040700.

Screw the environment. Please print this email immediately. And then burn it.

oakes commented 10 years ago

I added the new arity along with pixmap-format and pushed it as 0.4.1-SNAPSHOT. I haven't used pixmaps much so I don't have any examples. Is there a reason you can't just create a texture from it and render that?

joshuafcole commented 10 years ago

Well, it certainly was a newb question. I had already tried making a texture of it and draw!ing it to the screen, but at some point I'd jacked up the radius of the sample circle I was drawing to large values without similarly enlarging the pixmap. Since I was using :draw-circle instead of :fill-circle, the actual drawn pixels were just outside of the texture, so visually it looked like nothing was drawn.

Thanks for the advice, and thanks again for the speedy responses! play-clj definitely looks like an awesome framework, and I can't wait to share it with friends looking to learn game design once I'm a little more familiar.

Cheers

On Fri, Sep 26, 2014 at 9:02 PM, Zach Oakes notifications@github.com wrote:

I added the new arity along with pixmap-format and pushed it as 0.4.1-SNAPSHOT. I haven't used pixmaps much so I don't have any examples. Is there a reason you can't just create a texture from it and render that?

— Reply to this email directly or view it on GitHub https://github.com/oakes/play-clj/issues/51#issuecomment-57041552.

Screw the environment. Please print this email immediately. And then burn it.

oakes commented 10 years ago

No problem!

joshuafcole commented 10 years ago

Hmm, since we're unable to support the width/height/format ctor through the pixmap macro, is it possible to add a passthrough condition to the pixmap* function? E.g.

(pixmap 
  (pixmap* 100 100 (pixmap-format :RGBA8888))
  :set-color (color :red)
  :fill-circle 50 50 50)

;; This would be analagous to the existing behavior with texture.
(texture my-existing-texture
  :flip true false
  :set-region 0 0 100 100)

There is the possible gotcha that the Pixmap constructor from pixmaps seems to take Gdx2DPixmaps [1] rather than regular Pixmaps, so I'm not sure how you'd clone it. Even so, a straight passthrough would be useful and preferable to individual pixmap! calls.

Alternatively, is there an existing pattern I don't know about yet that I should prefer for this case? E.g., should I import play-clj.utils and manually invoke u/calls!?

Thanks

[1] http://libgdx.badlogicgames.com/nightlies/docs/api/com/badlogic/gdx/graphics/g2d/Gdx2DPixmap.html

joshuafcole commented 10 years ago

An alternate approach that would accomplish the same and potentially be more clear is to add a check in the pixmap macro itself to test whether the argument (currently path) is a Pixmap already, and if so just use it without invoking pixmap*

joshuafcole commented 10 years ago

Not sure if I ought to open a new bug for this, but in making a test case to determine the difference between using -> + pixmap! vs pixmap + options I discovered that the RGB* formats don't work with pixmap-format. It disregards the keyword casing, doing its own conversion, but the RGB* formats are acronyms where all the letters are uppercased. naively, removing the u/key->pascal call would fix that, but I expect you had it there so that beginners could just use the normal clojure casing conventions in the first place. Writing the keyword out as :r-g-b-a8888 works, but I think that's an even bigger gotcha.

http://libgdx.badlogicgames.com/nightlies/docs/api/com/badlogic/gdx/graphics/g2d/Gdx2DPixmap.html

Sorry for the trouble!

oakes commented 10 years ago

We could, though technically that would make it side-effecting. The texture macro creates a TextureRegion object from the Texture object, so it's a bit different. You should be able to do it like this:

(doto (pixmap* 100 100 (pixmap-format :RGBA8888))
  (pixmap! :set-color (color :red))
  (pixmap! :fill-circle 50 50 50))

Regarding the keyword issue, that will be documented when the next release happens. My documentation generator should display the case correctly; see, for example, the page for shape-type.

joshuafcole commented 10 years ago

And as a final addendum about the utility of a pixmap passthrough

    (let [pm (pixmap* 200 200 (pixmap-format :r-g-b-a8888))
          _ (doto pm
                 (pixmap! :set-color (color :white))
                 (pixmap! :fill)
                 (pixmap! :set-color (color :red))
                 (pixmap! :fill-circle 100 100 100))
          tex (texture pm)]
      (draw! screen [tex])
      (pixmap! pm :dispose))

VS.

    (let [pm (pixmap (pixmap* 200 200 (pixmap-format :RGBA8888))
                     :set-color (color :white)
                     :fill
                     :set-color (color :red)
                     :fill-circle 100 100 100)
          tex (texture pm)]
      (draw! screen [tex])
      (pixmap! pm :dispose))

It's definitely not the end of the world without it, but I do find the second form more attractive and less cluttered. Please forgive any typos, I just scratched them out into the github editor.

EDIT: Looks like you beat me to the doto example. :)