Cleanup and OpenGL 3 support for TextRenderer

[adbrown85]

Currently all the updates are in RasterTextRenderer, but will just need to do a rename to TextRenderer next. Figured it might be easier to review that way, but also really wanted to get this started today so you could look at it. I'll try to add some more comments about it tomorrow.

[sgothel]

I will go through your pull request and focus on feature duplication and namespace.
The whole set of pull requests looks great, kudos - much better than the previous one.

• ShaderLoader: Please use the already existing ShaderUtil, maybe ShaderCode/Program .. etc.
  It utilizes IOUtil from GlueGen and allows resource loading even on mobile.
  We constantly enhance it .. maybe you have ideas for it as well ..
• AWT Testing Utilities sidelines the junit folder and is a unique folder, ok.
  However, all code there relates to your 'special' testing environment
  hence I would like to see it in it's own subfolder, choose your magic subfolder/project name.
  Another one: I lack to see a use-case for all these testing classes in this pull request.
  W/o usage .. I don't like to take it .. it's like dead code.
• Glyph* and your Text* impl.
  I am sure it's great - I still have to check it :)
  One little sad thing here is it still uses AWT, which does not allow mobile platforms
  to benefit from your work.
  You might have seen, I have added TTF support
  in jogamp.graph.font.typecast.** (importet the typecast project and did a bit cleanup)
  What do you think about using typecast as the Glyph source instead of AWT ?

Sorry, I don't mean to procrastinate things here .. will keep an eye on it
and I hope I can try your code in 'a bit'.

The more your work is 'isolated' in it's own namespace denoting it's own sub-space,
the earlier we can merge. Later on - as you suggested, we may merge the code into the top-level
namespaces. Hopefully w/o AWT :)

[adbrown85]

I will go through your pull request and focus on feature duplication and
namespace. The whole set of pull requests looks great, kudos - much better
than the previous one.

Thanks!

ShaderLoader: Please use the already existing ShaderUtil, maybe ShaderCode/Program .. etc.
It utilizes IOUtil from GlueGen and allows resource loading even on mobile.
We constantly enhance it .. maybe you have ideas for it as well ..

I'll try to use them. At the time I was originally writing this it was easier to just load them myself since I only needed basic functionality, but I kept ShaderLoader package-protected with the goal of being able to replace it eventually.

AWT Testing Utilities sidelines the junit folder and is a unique folder, ok.
However, all code there relates to your 'special' testing environment
hence I would like to see it in it's own subfolder, choose your magic subfolder/project name.
Another one: I lack to see a use-case for all these testing classes in this pull request.
W/o usage .. I don't like to take it .. it's like dead code.

The testing utilites I added are used by GlyphRendererTest, QuadPipelineTest, and TextRendererTest in later commits. I just staged them separately to make it more digestible. I wasn't intending for the testing utilities to be public, they just resulted from realizing I was going to have a lot of duplicate code otherwise. I put everything in the awt package because GlyphRenderer and QuadPipeline are package-protected, and I wouldn't be able to test them separately otherwise.

One little sad thing here is it still uses AWT, which does not allow mobile platforms
to benefit from your work.
You might have seen, I have added TTF
support in jogamp.graph.font.typecast.** (importet the typecast project and did a bit cleanup)
What do you think about using typecast as the Glyph source instead of AWT ?

I think everything is broken out pretty well now, so that the changes that would need to be made would be relatively isolated, compared to what was there before at least.

However, having talked to Jason it sounds like we probably won't be able to justify spending much money on that. In general I guess we were just intending to keep TextRenderer usable as it was, except with OpenGL 3/4 support.

Maybe if we could get the conversion a little more concrete though... Is there a way with the typecast project to rasterize the font into a BufferedImage?

[adbrown85]

Ok, finally cleaned up and pushed the commit where I renamed RasterTextRenderer to TextRenderer. I'll work on cleaning up the shader loading, but having done that I'm comfortable in saying this could probably be pulled as is for now considering not much has changed publicly. The only thing I added to its interface was setTransform, since we don't have OpenGL's internal transformation stack in the core profile, and a constructor taking a UnicodeBlock, which is sort of unnecessary but it does allow for some optimization/simplification.

[sgothel]

On 04/23/2012 05:13 PM, Andy Brown wrote:

I will go through your pull request and focus on feature duplication and
namespace. The whole set of pull requests looks great, kudos - much
better than the previous one.

Thanks!

ShaderLoader: Please use the already existing ShaderUtil, maybe
ShaderCode/Program .. etc. It utilizes IOUtil from GlueGen and allows
resource loading even on mobile. We constantly enhance it .. maybe you
have ideas for it as well ..

I'll try to use them. At the time I was originally writing this it was
easier to just load them myself since I only needed basic functionality,
but I kept ShaderLoader package-protected with the goal of being able to
replace it eventually.
Good, so we may earmark this for a later cleanup.

AWT Testing Utilities sidelines the junit folder and is a unique folder,
ok. However, all code there relates to your 'special' testing
environment hence I would like to see it in it's own subfolder, choose
your magic subfolder/project name. Another one: I lack to see a use-case
for all these testing classes in this pull request. W/o usage .. I don't
like to take it .. it's like dead code.

The testing utilites I added are used by GlyphRendererTest,
QuadPipelineTest, and TextRendererTest in later commits. I just staged
them separately to make it more digestible. I wasn't intending for the
testing utilities to be public, they just resulted from realizing I was
going to have a lot of duplicate code otherwise.
Ok, so we can (later on) create a sub-package for it to emphasize their
intention for testing, good.

I put everything in the
awt package because GlyphRenderer and QuadPipeline are
package-protected, and I wouldn't be able to test them separately
otherwise.
The public / private separation in JOGL doesn't necessarily rely on
the Java module right now, but we simply indicate this by the package
namespace.

'com.jogamp.' is public, i.e. intended for public use, where
'jogamp.' is semi private, i.e. not intended for public use
but holds implementations.

I personally use private/protected/package-protected as well as final
qualifiers as well, but more intended to clarify the intention and visibility.
The access right qualifiers can't be used properly for security anyways,

In the 'dawn' of a future AWT-less implementation we may prepare the
namespaces (package names) accordingly, at least I would like to do that
if you don't mind.

One little sad thing here is it still uses AWT, which does not allow
mobile platforms to benefit from your work. You might have seen, I have
added TTF support in jogamp.graph.font.typecast.** (importet the typecast
project and did a bit cleanup) What do you think about using typecast as
the Glyph source instead of AWT ?

I think everything is broken out pretty well now, so that the changes that
would need to be made would be relatively isolated, compared to what was
there before at least.
Of course! I don't compare against the 'old' quality here :)

However, having talked to Jason it sounds like we probably won't be able to
justify spending much money on that. In general I guess we were just
intending to keep TextRenderer usable as it was, except with OpenGL 3/4
support.
I see. Maybe I will go through such a task, since for my and colleagues
we consider an AWT only impl. to be broken. Meaning we love to see things
on mobile w/o the need for AWT. This also includes GLES2 ofc.
But I don't know when I have time for this either, so I guess merging your
work now seems to be a good time.

Maybe if we could get the conversion a little more concrete though... Is
there a way with the typecast project to rasterize the font into a
BufferedImage?

The latter is AWT as well IMHO.
If you require the curves rasterized we would need an implementation
for it - right, you cache the current AWT font rasterization
and use textures ? Sorry for being no expert of the details here (yet).

Bottom line, as you said, you replace the previous font renderer
with a better implementation and added functionality regarding the used
GL profiles. This alone should let us finally merge.

Kudos!

~Sven

[adbrown85]

Ok, this doesn't exactly inspire confidence but I ran the the test for TextRenderer that was in there before and realized I introduced a NullPointerException when the user calls setUseVertexArrays before beginRendering. Working on fixing that now.

[adbrown85]

Ok, fixed the bug with setUseVertexArrays, and one with dispose, too. Cleaned up ShaderLoader a bit, where now we pass it a GL, validate the program, and at least most of the heavy functionality kicks to ShaderUtil.

[ghost]

Does this text renderer still support OpenGL 1.1? Sorry for my silly question.

[adbrown85]

Turns out not so silly... I was sure I saw somewhere that a few of the glPixelStore parameters were only in OpenGL 1.2, so I had a check in there for that, but I just went back to make sure and couldn't find that again. It looks like it was spelled out a little more explicitly in the 1.2 spec though so maybe that's why. Anyway, the short answer is yes (now) it should support 1.1 again. Thanks for asking!

[ghost]

It would be nice if this pull request was merged as some users are complaining about the inability of using the legacy text renderer with GL3.

[adbrown85]

Spent awhile cleaning it up over the weekend. Would like to do a bit more but ran out of time. Monday nights are usually pretty open for me though so I might be able to finish it up tonight.

[ghost]

I'd like to see this pull request in JOGL 2.3.2 or 2.4. I'll talk about it to @sgothel

[adbrown85]

Awesome! I have one outstanding change still, which was to use JUnit rules
to run my tests on the Swing thread because they're a little wonky / messed
up right now, but I guess that could be a separate pull request too. So
far I was just testing it in a separate project so I'm not sure if it'll
work with JOGL's version of JUnit, etc. though.

On Wed, Aug 26, 2015 at 10:50 AM, Julien Gouesse notifications@github.com
wrote:

I'd like to see this pull request in JOGL 2.3.2 or 2.4. I'll talk about it
to @sgothel https://github.com/sgothel

—
Reply to this email directly or view it on GitHub
#47 (comment).

[ghost]

Is this change absolutely necessary? Yes, rather put it into a separate pull request. Please bump Sven on IRC, it would be nice that you talk about your changes to him. In my humble opinion, your code shouldn't be wasted, this is something that I would like to see in JOGL, really.

[sgothel]

On 08/27/2015 03:50 PM, Andy Brown wrote:

Awesome! I have one outstanding change still, which was to use JUnit rules
to run my tests on the Swing thread because they're a little wonky / messed
up right now, but I guess that could be a separate pull request too. So
far I was just testing it in a separate project so I'm not sure if it'll
work with JOGL's version of JUnit, etc. though.

Unit tests within our framework would be more than welcome - of course!

Further, since Julien seems to jump in here, voting for a merge,
I ask Julien: Can you please manually test this branch in regards
to this work? Hint: Regressions.

If Julien gives me the green light (after manually validating no regression)
and if your (Andy) unit tests arrive and pass - I gladly will merge!

@julien: Since this is the first commit of Andy,
we would also need to review for possible security issues as well.
Usually we always should do that, of course.
Hint: Any new or modified privileged code, e.g.:

• 'AccessController.doPrivilege'
• JNI code
  should raise concerns of course.

I know, previous code base had almost no unit tests,
but a simple 'black box' one(s).
Still - since you have unit tests, they would be more than appreciated!

Even though this list might seem a bit long,
thank you Andy for catching up with your 2-3 year (?) long work here.

~Sven

[julien]

Hi guys, I think it's @gouessej not @julien (that's me) ;)

[paolofuse]

I'm using this version of TextRenderer for GL3 on Mac OSX but I had to fix some bugs:

• the use of VAO was forced, I have made it working with setUseVertexArrays method of TextRenderer (classes GlyphRendererGL3, QuadPipelineGL30)
• the setColor method set wrong color, using Green component also for Blue (method doSetColor of GlyphRendererGL3)
• calling beginRendering for the first time with a not white (1, 1, 1, 1) color threw an exception, I have created a init() method to call before start rendering (maybe this method is not necessary, maybe there's a better solution)

Then it works perfectly.

[adbrown85]

@paolofuse Awesome!

Can you make a fork for me to pull in your changes? Or send me some patches from format-patch?

Also, can you explain why you don't want to or can't use a VAO with 3? You're not using a core profile I take it? It's been awhile and I don't do a lot of OpenGL anymore but I think the reason it's forcing it is I thought VAOs are required in core. If we do still allow it should we put a check in for core? Can't remember is JOGL GL3 supposed to always be core? I guess GL3bc or something like that extends GL3?

[adbrown85]

@paolofuse Is the init method something the user calls on TextRenderer? Or is it on one of the helper classes?

[ghost]

@adbrown85 GL3bc extends GL2 and GL3, it means "GL3 backward compatible. Therefore, you can get a GL3 instance that isn't core, use GLBase.isGL3core() to check that. Keep in mind that there is a default VAO created under the hood in JOGL.

[paolofuse]

Hi all,
here you can find the fork from adbrown85 branch:
https://github.com/fusedeveloper/jogl

Feel free to comments my solution.
Thanks

[paolofuse]

@adbrown85 the init method must be call by the user before call beginRenderer.

[jedwards1211]

@adbrown85 @gouessej what's the status on this?

I want to use this because I can't get TextRegionUtil to work properly.

[ghost]

@jedwards1211 Sorry for the delay. You should ask for help on the forum concerning TextRegionUtil. We'll try to integrate this stuff in JOGL 2.4.0 if we have enough time to fix and test it anew.

[jedwards1211]

@gouessej okay, thanks. I ended up just using code I copied out of @paolofuse's fork for text. It's serving my needs well enough for now, though I may want geometric text in the future.

[ghost]

@jedwards1211 Can you fix the conflicts so that we can integrate it faster?

[jedwards1211]

@gouessej sure! I should have time to do so this weekend.

[ghost]

Ok let me know when you're ready.

[jedwards1211]

Sorry, yeah, I've gotten sidetracked...

[jzy3d]

Hi!

Thank you @adbrown85 for this work! I tested it in Jzy3D and it provides great improvements

• font look more accurate
• memory footprint is smaller

In situation where I introduced memory leak by mistake in Jzy3D, I have been able to double the time before a JVM crash occurs when switching to your TextRenderer. My real bug was my own code, but on my way I tried your work and it is really great.

I copy pasted your code here and even deployed on my Maven repo for those willing to use at well.

That would be nice it was merged on @sgothel branch for a later deployment, otherwise I'll to add it to my JOGL clone if anyday I rebuild JOGL fully.