Skip to content

Add antialiasing toggle to text tool#935

Merged
cameronwhite merged 5 commits into
PintaProject:masterfrom
potatoes1286:master
Aug 22, 2024
Merged

Add antialiasing toggle to text tool#935
cameronwhite merged 5 commits into
PintaProject:masterfrom
potatoes1286:master

Conversation

@potatoes1286

Copy link
Copy Markdown
Contributor

I was frustrated with the lack of ability to disable antialiasing on Pinta, so I decided to just go and add it.

Please note this was cobbled together. I have zero experience with Cairo/Pango/PangoCairo but in my brief testing there didn't seem to be any obvious problems.

Thank you in advance.

@cameronwhite

Copy link
Copy Markdown
Member

Thanks for having a look! I'll need to have a look at the pango / Cairo stuff and do some testing, but for the toolbar button / settings I think it would be best to reuse the standard antialiasing button from the base class ( BaseTool). I think you just need to override ShowAntialiasingButton and then check the UseAntialiasing property when drawing

@potatoes1286

potatoes1286 commented Aug 11, 2024

Copy link
Copy Markdown
Contributor Author

Thanks for having a look! I'll need to have a look at the pango / Cairo stuff and do some testing, but for the toolbar button / settings I think it would be best to reuse the standard antialiasing button from the base class ( BaseTool). I think you just need to override ShowAntialiasingButton and then check the UseAntialiasing property when drawing

This is possible, but as redraw is only called when a change is made, if you change the anti-aliasing setting before text finalization, the change does not register until another action is performed (e.g moving cursor, adding new text to text, etc.)

@cameronwhite

Copy link
Copy Markdown
Member

Right, that makes sense. I think it would be reasonable to add an event in BaseTool for this so that re-editable tools can handle the antialiasing setting changing and trigger their own redraw.

@potatoes1286

Copy link
Copy Markdown
Contributor Author

@cameronwhite Added this functionality to BaseTool, and readjusted the text tool to support it.

Comment thread Pinta.Core/Classes/BaseTool.cs Outdated
Comment thread Pinta.Tools/Tools/TextTool.cs Outdated

protected override void OnChangeAntialias (object? sender, EventArgs e)
{
UpdateFont();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that after I finished typing some text (i.e. after pressing Esc) that the antialiasing changes didn't cause a redraw right away until I did something else to force a redraw, like pressing Ctrl.

This also happens with other font properties too, though, so it's a more general behaviour and could probably be left for another pull request

@potatoes1286 potatoes1286 Aug 22, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it seems like in that state after pressing escape, the text is not considered in edit mode, and so does not call redraw.

The if (is_editing) statement on UpdateFont in TextTool prevents the redraw. It seems to be there for a reason (removing it then starting Pinta with text tool on causes crash), so i won't touch it for the time being in case it causes other issues.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've filed a separate bug #952 for this

Comment thread Pinta.Tools/Tools/TextTool.cs Outdated
Comment thread Pinta.Tools/Tools/TextTool.cs Outdated
@cameronwhite

Copy link
Copy Markdown
Member

The changes look good to me - thanks!
The CI build is just failing from the code formatting check - you should be able to run dotnet format to fix that

@cameronwhite cameronwhite merged commit 5d32780 into PintaProject:master Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants