AuburnSounds / Dplug

Audio plugin framework. VST2/VST3/AU/AAX/LV2 for Linux/macOS/Windows.
https://dplug.org/
Other
486 stars 32 forks source link

Font crash/issues with partial updates #642

Closed PolyVector closed 2 years ago

PolyVector commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Thanks. I'll look at it next week.

PolyVector commented 2 years ago

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! :)

PolyVector commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.