PistonDevelopers / gfx_text

Draw text for gfx using freetype
http://docs.piston.rs/gfx_text/gfx_text/
MIT License
22 stars 12 forks source link

Stream api #11

Closed Kagami closed 9 years ago

Kagami commented 9 years ago

Fixes #4 cc @kvark

kvark commented 9 years ago

I think the stream version should be the default, and s prefix is not too sound. Canvas API can follow gfx-debug-draw case (draw_end_canvas). @stjahns would you see it fit?

Kagami commented 9 years ago

I think the stream version should be the default

That would be a breaking change. Not a big deal at this level of development though, so it's ok.

Canvas API can follow gfx-debug-draw case (draw_end_canvas)

Ok, we may be consistent with gfx-debug-draw here.

Would like to hear @stjahns opinion too.

Kagami commented 9 years ago

Seems like @stjahns is quite inactive last time and one of the library users waiting for this.

I've rebased over master, made draw_end[at] accept the stream by default and also add `canvas` aliases instead because it's more logical IMO.

@kvark what do you think?

kvark commented 9 years ago

@Kagami It's a bit unfortunate to see this much boilerplate just to support Canvas. I believe the user would need to change less/nothing in case they need it:

  1. for simple apps, like the examples here, it just adds one line to have a (let mut stream = (&mut canvas.renderer, &convas.output);)
  2. more complex apps are not supposed to send Canvas all the way down to the rendering routines anyway.

Hence, I'm leaning towards gfx_text providing only Stream API. The burden of extracting the stream will be removed with https://github.com/gfx-rs/gfx-rs/issues/690, which would allow gfx_text to have its own factory.

@LaylConway - would it hurt your use case?

Kagami commented 9 years ago

Ok, fixed. Does it look good to you now?

@LaylConway actually wanted stream API, so it should be ok.

LaylBongers commented 9 years ago

I'm personally of the opinion that Canvas is an unneeded abstraction now that Stream is provided. I've been using this fork so far in Phosphorus and it seems to work great for my needs.

kvark commented 9 years ago

@LaylConway thanks for the feedback!