Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Font crash/issues with partial updates #642

Closed
PolyVector opened this issue Apr 15, 2022 · 7 comments
Closed

Font crash/issues with partial updates #642

PolyVector opened this issue Apr 15, 2022 · 7 comments
Labels
Bug Reproduced bug.

Comments

@PolyVector
Copy link

PolyVector commented Apr 15, 2022

Hello,
I've been seeing crashes caused by fillText.drawCharacter() in font.d, and think I've narrowed down the cause.
it appears that surface.cropImageRef() can be called with zero area when only a section of text is redrawn.

Here's a quick fix that has solved my issue:

       // follows the cropping limitations of crop()
        int cropX0 = clamp!int(offsetPos.x, 0, surface.w);
        int cropY0 = clamp!int(offsetPos.y, 0, surface.h);
        int cropX1 = clamp!int(offsetPos.x + w, 0, surface.w);
        int cropY1 = clamp!int(offsetPos.y + h, 0, surface.h);
        
        // FIX: Avoid crash on partial updates
        if (cropX1<=cropX0 || cropY1<=cropY0)
        	return;
        	
        auto outsurf = surface.cropImageRef(cropX0, cropY0, cropX1, cropY1);

Also related is this function appears to garble characters when partially redrawn. The cause appears to be reading from the wrong position in the character. For reference, here is my embarrassingly hacky fix. I'm sure someone with more familiarity with Dplug surfaces could do a proper crop in a line or two:

        int offsetX = 0;
        if (offsetPos.x < 0)
        	offsetX = -offsetPos.x;
        int offsetY = 0;
        if (offsetPos.y < 0)
        	offsetY = -offsetPos.y;
        for (int y = 0; y < outsurf.h; ++y)
        {
            RGBA[] outscan = outsurf.scanline(y);
			int inY = y+offsetY;
			if(inY < 0 || inY >= h)
				continue;
            L8[] inscan = coverageBuffer.scanline(inY);
            for (int x = 0; x < croppedWidth; ++x)
            {
            	int inX = x+offsetX;
            	if(inX < 0 || inX >= w)
            		continue;
                blendFontPixel(outscan.ptr[x], fontColor, inscan.ptr[inX].l);
            }
        }

I'm new to Dplug and it appears to be quite powerful. Thanks for all your hard work, hopefully this is helpful.

@p0nce
Copy link
Collaborator

p0nce commented Apr 15, 2022

Hello, how should I reproduce the problem? It's good to provide a solution, but without being able to reproduce the issue it's hard to know if this is the right fix.

@PolyVector
Copy link
Author

PolyVector commented Apr 15, 2022

Sorry about that. Here's a quick UIElement that should crash if you click around on it enough. It's a label based on the PBR label that ships with Dplug, only it marks a pixel as dirty if you click on it.

module testlabel;

import gui;
import core.atomic;
import dplug.core;
import dplug.gui;
import dplug.client;
import dplug.graphics.font;
import std.math;
import std.algorithm.comparison;

class UITestLabel : UIElement
{
public:
nothrow:
@nogc:

this(UIContext context, Font font, const(char)[] text, RGBA color = RGBA(255,255,255,255))
{
	super(context, flagRaw);
	_font = font;
	_text = text;
	fontColor = color;
}

// BUG DEMONSTRATION: Click or Drag and the label should garble or crash
void demonstrateBug(int x, int y)
{
	int dirtyX0 = clamp!int(x-5, 0, position.size.x);
	int dirtyY0 = clamp!int(y-5, 0, position.size.y);
	int dirtyX1 = clamp!int(x+5, 0, position.size.x);
	int dirtyY1 = clamp!int(y+5, 0, position.size.y);
	if (dirtyX1>dirtyX0 && dirtyY1>dirtyY0)
		setDirty(box2i(dirtyX0, dirtyY0, dirtyX1, dirtyY1));
}
override bool onMouseClick(int x, int y, int button, bool isDoubleClick, MouseState mstate)
{
	demonstrateBug(x, y);
	return true;
}
override void onMouseDrag(int x, int y, int dx, int dy, MouseState mstate)
{
	demonstrateBug(x, y);
}

override void onDrawRaw(ImageRef!RGBA rawMap, box2i[] dirtyRects)
{
        float textPosx = position.width * 0.5f;
        float textPosy = position.height * 0.5f;
		
       foreach(dirtyRect; dirtyRects)
        {
            auto croppedRaw = rawMap.cropImageRef(dirtyRect);
            vec2f positionInDirty = vec2f(textPosx, textPosy) - dirtyRect.min;
            croppedRaw.fillText(_font, _text, position.height, 0, fontColor, positionInDirty.x, positionInDirty.y);
        }
}

private:
	Font _font;
	const(char)[] _text;
	@ScriptProperty RGBA fontColor = RGBA(255, 255, 255, 255);
}

Also, this still crashes with my "fix", so I suppose I've only solved one specific edge case my project was running into. In my particular case I was positioning LED-style indicators using wren, and the plugin would crash if they touched a label.

Edit: Updated this demonstration code to better showcase the issue (and not cause issues itself).

@p0nce p0nce added the Bug Reproduced bug. label Apr 15, 2022
@p0nce
Copy link
Collaborator

p0nce commented Apr 15, 2022

Thanks. I'll look at it next week.

@PolyVector
Copy link
Author

Okay, this was nagging at me because my fix was working in my plugin, but not the repro class above... Apparently I just botched the repro and set the dirty rects wrong... I've updated the class above to fix that and better showcase the issue.

Sorry for the bump, have a great weekend! :)

p0nce pushed a commit that referenced this issue Apr 17, 2022
Not sure if this is the real issue #642 or something else.
@PolyVector
Copy link
Author

PolyVector commented Apr 17, 2022

That fixed the crashing for me, thanks :)

I improved my own fix for the garbled rendering portion of the issue, here it is for reference:

        RGBA fontColor = textColor;

        // Fix garbled rendering
        int coverageOffsetX = (offsetPos.x < 0) ? -offsetPos.x : 0;
        int coverageOffsetY = (offsetPos.y < 0) ? -offsetPos.y : 0;
        // Early exit if nothing to render
        if (coverageBuffer.w-coverageOffsetX <= 0 || coverageBuffer.h-coverageOffsetY <= 0)
        	return;
        // Crop Glyph to match Output
        if (coverageOffsetX > 0 || coverageOffsetY > 0)
                coverageBuffer = coverageBuffer.cropImageRef(coverageOffsetX, coverageOffsetY, coverageBuffer.w, coverageBuffer.h);
        // Crop Output to match Glyph
        if (coverageBuffer.w != outsurf.w || coverageBuffer.h != outsurf.h) // Crop Output to match Glyph?
                outsurf = outsurf.cropImageRef(0,0,min(coverageBuffer.w, outsurf.w), min(coverageBuffer.h, outsurf.h));

        for (int y = 0; y < outsurf.h; ++y)
        {
            RGBA[] outscan = outsurf.scanline(y);

            L8[] inscan = coverageBuffer.scanline(y);
            for (int x = 0; x < croppedWidth; ++x)
            {
                blendFontPixel(outscan.ptr[x], fontColor, inscan.ptr[x].l);
            }
        }

@p0nce
Copy link
Collaborator

p0nce commented Apr 17, 2022

Hello,

I've reproduced both your issue, indeed my commit fix the first one (your solution would have worked too), and your second patch does indeed fix a bad drawing in drawCharacter.

I'll push soon something similar, or perhaps your exact patch.

Thanks you very much!

@p0nce p0nce closed this as completed in 194ff3c Apr 17, 2022
@p0nce
Copy link
Collaborator

p0nce commented Apr 17, 2022

Hello,

You can update to v12.5.1 with the fix.
The issue was indeed character rendering in the case where the character it partially outside the output surface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Reproduced bug.
Projects
None yet
Development

No branches or pull requests

2 participants