DisplayString optimization and bug

[Aphexus]

trafficstars

For some reason DisplayString no longer works(just did)... Not sure why... probably on my side.

But what I noticed was that it always create a new image to hold the string.

This seems like it would be excessive in wasting memory, causing the GC to run a lot, etc.

I tried caching the strings but I was still getting the crash(was working in a demo app and this app but then stopped working once I added other code completely unrelated(but all the graphics work).

Of course, I get the dreaded 1282.

But more importantly, surely you could figure out a way to not have to create a new memory image each time for each frame?

Could one not render on a large image(and if not large enough resize like we do with arrays). One basically renders on to the image at some free location that can fit the text and return the sub image to use.

This way the problem scales with the usage. Using very small bits of text should not require any new image allocations.

Maybe allocator could be used somehow since basically it is a memory problem... might reduce the work significantly... or maybe there is a better way?

[adamdruppe]

On Thu, Oct 11, 2018 at 03:36:11AM -0700, Aphexus wrote:

For some reason DisplayString no longer works(just did)... Not sure why... probably on my side.

You mean the OpenGlTexture constructor in gamehelpers.d? http://dpldocs.info/experimental-docs/arsd.gamehelpers.OpenGlTexture.this.2.html

You are supposed to hold on to those handles in your code, that's why only the constructor does work.

The analogous function in simpledisplay (for use without opengl) does have an internal cache.

[Aphexus]

Ok, I thought it was part of your library but I must have copied it from someone and stuck it in there

static class InitData
{
	static TtfFont Font;
}

static this()
{
	InitData.Font = TtfFont(cast(ubyte[])read("C:\\Windows\\Fonts\\arial.ttf"));	
}

OpenGlTexture[string] textImagesCached;
void DrawString(string text, int s, int x, int y, byte r = cast(byte)255, byte g = cast(byte)255, byte b = cast(byte)255, byte br = 0, byte bg = 0, byte bb = 0)
{
	if (text in textImagesCached)
	{
		auto image = textImagesCached[text];
		image.draw(x, y, image.Width, image.Height);
		return;
	}

	auto font = InitData.Font;	

	int w, h;
	auto bitmap = font.renderString(text, s, w, h);
	auto image = new Image(w, h);

	for (int j=0; j < h; ++j) 
	{
		for (int i=0; i < w; ++i)
			if (bitmap[j*w+i] > 128)
				image.putPixel(i, j, Color(r, g, b));
			else
				image.putPixel(i, j, Color(br, bg , bb));
	}
	auto oGLimage = new OpenGlTexture(image.toTrueColorImage());
	oGLimage.draw(x, y, w, h);		
}	

I guess one doesn't need to create texture and draw it but can write directly on the buffer/screen or maybe an image overlay for the screen is required that can be used to draw text and 2D graphics to that i allocated only once and cleared per frame? Not really sure about this because I know very little about OpenGL.

Maybe you could add such a function to your code so that it is easier to draw strings in opengl, which is basically a need for most apps.

[Aphexus]

Ok, I was able to get it to work using a preallocated image but I have to create a new OpenGlTexture of it each time.

As long as that isn't too heavy then the only problem I'm having is that when i draw the image, it covers everything. Seems the alpha channel isn't working ;/

e.g., I initially do

	for (int j=0; j < Height; ++j) 
		for (int i=0; i < Width; ++i)
			ImageOverlay.putPixel(i, j, Color(0,0,0,255));

(or 255 => 0) or Color.transparent

but still overwrites the entire screen. Also, it still throws an error as mentioned in another thread... but I commented that out and it works. Everything works now except the error(meaning something is wrong, but which I have no clue) and not being able to have it draw transparently so an overlay can be achieved.

Note that it is only asserting on this draw string code(pasted earlier) because I have other image drawing stuff(using same gamehelpers routine) and it works fine.

[Aphexus]

After getting everything to "work". I quickly get an out of memory error! ;/

occurs here:

	auto oGLimage = new OpenGlTexture(image.toTrueColorImage());

from TrueColorImage toTrueColorImage() { auto tci = new TrueColorImage(width, height); convertToRgbaBytes(tci.imageData.bytes); return tci; }

as it is allocating an entirely new image every frame, the size of the window.

It would just be really nice to write directly on the openGL screen buffer and avoid all this! ;/

If this is required to be able to display strings then it is dysfunctional. One cann't allocate new buffers like this every frame to simply be able to draw text. I tried to create the TrueColorImage first but then nothing is show.

I converted the put pixels to use setPixel from the true color image and things seem to work now. Although the app seems slightly more laggy and uses a constant 15% of cpu.

The good news is that the transparency then works.

But I eventually get an assert error from here:

// gotta round them to the nearest power of two which means padding the image
	if((_texWidth & (_texWidth - 1)) || (_texHeight & (_texHeight - 1))) {
		_texWidth = nextPowerOfTwo(_texWidth);
		_texHeight = nextPowerOfTwo(_texHeight);

		auto n = cast(ubyte*) calloc(_texWidth * _texHeight * 4,1);
		if(n is null) assert(0);

[adamdruppe]

On Fri, Oct 12, 2018 at 12:40:03AM -0700, Aphexus wrote:

Ok, I thought it was part of your library but I must have copied it from someone and stuck it in there

This is what the lib provides:

http://dpldocs.info/experimental-docs/arsd.gamehelpers.OpenGlTexture.this.2.html

You can call that like

auto txt = new OpenGlTexture(&Font, size, "Some string");

then just draw that wherever. You should store that at some high level so you can reuse it, like you did with the associative array.

If you are displaying things that rapidly change but are a small number of letters, like a score, you might cache the digits individually and space them yourself. This is hard to do for proportional fonts like Arial.. but you could use a monospace font for the score display though, then each letter is advanced a certain number of pixels.

I guess one doesn't need to create texture and draw it but can write directly on the buffer/screen

So that is possible, but not with the library code I have. You'd have to turn the font commands into opengl commands. My libs don't support that.

or maybe an image overlay for the screen is required that can be used to draw text and 2D graphics to that i allocated only once and cleared per frame? Not really sure about this because I know very little about OpenGL.

that won't work tho.

[Aphexus]

What is the point of having to jump through several hoops to simply blit one image on another? Why can I use one image as a scratch pad, draw the font on it, then blit that image to open gl?

Seems like the font rendering functions you provide should be able to take an opengl image and bit to them. I could care less about the speed, within reason, but having to break it up in to parts just to render dynamically changing text is a bit too much. The whole point of having a library is to ease the burden of having to do low level work like this.

Since you've done much of the work surely it wouldn't be that difficult to get the fond renderer to work on an openGL texture somehow? Even if it requires transferring in to main memory from gpu and vice versa?

[adamdruppe]

On Sun, Oct 14, 2018 at 03:48:47AM -0700, Aphexus wrote:

What is the point of having to jump through several hoops to simply blit one image on another? Why can I use one image as a scratch pad, draw the font on it, then blit that image to open gl?

That's what the library does.

[jfhs]

There is actually a leak in renderString:

auto c = renderCharacter(ch, size, cw, cheight, x_shift, 0.0);

Calls stbtt_GetCodepointBitmapSubpixel->stbtt_GetGlyphBitmapSubpixel which does

gbm.pixels = cast(ubyte *) STBTT_malloc(gbm.w * gbm.h, info.userdata);
//...
return gbm.pixels;

All the way back to renderCharacter which takes a slice on it and returns to renderString. This array is never freed.

Adding something like:

stbtt_FreeBitmap(cast(ubyte*)c, font.userdata);

After foreach which uses c resolves the issue, however that's still a lot of allocs/deallocs for rendering a string. I wonder if there are any optimizations possible (except not using renderString a lot :smile:)

I guess that's what @Aphexus is hitting since renderString is also used in OpenGlTexture constructor from string.

[adamdruppe]

On Mon, Oct 15, 2018 at 03:01:28PM -0700, Andrey wrote:

There is actually a leak in renderString:

Oh yeah, yikes, that's legit. Just pushed a free.

After foreach which uses c resolve issue, however that's still a lot of allocs/deallocs for rendering a string, I wonder if there is any optimizations possible (except not using renderString a lot :smile:)

Yes, another possibility would be to make it use a region allocator or something to reuse a buffer. Or really digging into the stb_truetype code and doing lots of changes, like passing it a buffer. (Though there are a LOT of internal mallocs in other functions too, so maybe the reusable region allocator is the way to go.)

I guess that's what @Aphexus is hitting since renderString is also used in OpenGlTexture constructor from string.

Right. Just then if it is reused it isn't so crazy.

[adamdruppe]

so i tried moving the internal allocations to a stack region...

and it made no difference. malloc and free are already pretty reasonable on these little things.

[adamdruppe]

With the memory leak fixed, I also added methods to reuse an OpenGlTexture and to free them manually, instead of waiting on the GC.

Now it is possible to write code like this:

        auto texture = new OpenGlTexture();
        window.redrawOpenGlScene = () {
                window.clearOpenGlScreen();
                texture.dispose();
                texture.bindFrom(&font, 16, buffer[]);
                texture.draw(10, 10);
	}

Which reuses the one class and rebinds it to a new string. Seems to have no memory leak, I ran this 10,000 times and saw no opengl problem (though I grant that doesn't mean there isn't one), and the benchmark shows ok perf.

There are still optimization opportunities in there - like the ctor that takes the buffer from ttf.d and passes it to opengl still copies and news a few things that aren't strictly necessary - but it seems to work.

Lemme know if this is any better to yall.

[Aphexus]

Here is a renderstring that uses opengl to render to the screen directly. You might want to add it to your library as it provides a method to render and doesn't use any memory:

auto renderString2Screen(ref TtfFont font, in char[] s, int xx, int yy, int size, out int width, out int height, ubyte rr = 255, ubyte gg = 255, ubyte bb = 255) { float xpos=0;

auto scale = stbtt_ScaleForPixelHeight(&font.font, size);
int ascent, descent, line_gap;
stbtt_GetFontVMetrics(&font.font, &ascent,&descent,&line_gap);
auto baseline = cast(int) (ascent*scale);

import std.math;

int swidth;
int sheight;
font.getStringSize(s, size, swidth, sheight);

glPointSize(0.95);  
glBegin(GL_POINTS);
glColor3f(1,1,1);
int dy = 0;
foreach(i, dchar ch; s) {
	
	if (ch == '\n')
	{
		xpos = 0;
		dy += baseline + cast(int)(scale*line_gap);
		continue;
	}
	int ddy = dy;
	int advance,lsb;
	auto x_shift = xpos - floor(xpos);
	stbtt_GetCodepointHMetrics(&font.font, ch, &advance, &lsb);
	int cw, cheight;
	auto c = font.renderCharacter(ch, size, cw, cheight, x_shift, 0.0);

	int x0, y0, x1, y1;
	stbtt_GetCodepointBitmapBoxSubpixel(&font.font, ch, scale,scale,x_shift,0, &x0,&y0,&x1,&y1);

	int x = cast(int) xpos + x0;
	int y = baseline + y0 + dy;
	int cx = 0;
	
	
	foreach(index, pixel; c) {
		if(cx == cw) {
			cx = 0;
			y++;
			x = cast(int) xpos + x0;
		}
		auto offset = swidth * y + x;


		
		if (pixel > 0)
		{
			glColor4f(pixel*rr/255.0/255.0,pixel*gg/255.0/255.0,pixel*bb/255.0/255.0,pixel/255.0);
			glVertex2i(xx+x,yy+y);
		}

		x++;
		cx++;
	}
	

	//stbtt_MakeCodepointBitmapSubpixel(&font, &screen[(baseline + y0) * swidth + cast(int) xpos + x0], x1-x0,y1-y0, 79, scale,scale,      x_shift,0, ch);
	// note that this stomps the old data, so where character boxes overlap (e.g. 'lj') it's wrong
	// because this API is really for baking character bitmaps into textures. if you want to render
	// a sequence of characters, you really need to render each bitmap to a temp buffer, then
	// "alpha blend" that into the working buffer
	xpos += (advance * scale);
	if (i + 1 < s.length)
		xpos += scale*stbtt_GetCodepointKernAdvance(&font.font, ch,s[i+1]);
}
glEnd();


width = swidth;
height = sheight;

}