Add outline option for rendering fonts #2828

narilee2006 · 2024-04-26T21:11:53Z

Hello! I tried doing #2452. Let me know if there are any issues.

src_c/font.c

buildconfig/stubs/pygame/font.pyi

src_c/font.c

test/font_test.py

oddbookworm

I have just a couple comments

test/font_test.py

oddbookworm

LGTM!

Starbuck5

The challenge of #2452 is to design a good API for this, and this PR doesn’t accomplish that in my opinion.

Outline color isn’t supported
Looking at my notes from before (haven’t run this yet), doesn’t this PR mean that people have to call font.render twice? Once for the text, then once for the outline. That seems inconvenient for users.

narilee2006 · 2024-04-27T16:07:27Z

Outline color isn’t supported

It actually is, you need to call it in the font.render() method.

Looking at my notes from before (haven’t run this yet), doesn’t this PR mean that people have to call font.render twice? Once for the text, then once for the outline. That seems inconvenient for users.

Yep, that's how SDL2 implemented it.

narilee2006 · 2024-04-27T16:19:16Z

Perhaps outline as a kwarg so the developer doesn't need to use two lines to create the font?

oddbookworm · 2024-04-27T16:23:35Z

Maybe perhaps

pygame.font.Font(filename=None, size=20, outline=0)

for the font constructor and

pygame.font.Font.render(text, antialias, color, bgcolor=None, wraplength=0, outline_color=None)

for the render call?

But I'm not sure what to do if outline_color defaults when outline was set to something nonzero

Starbuck5 · 2024-04-27T16:27:01Z

Perhaps outline as a kwarg so the developer doesn't need to use two lines to create the font?

Well then how do you do the color? Another kwarg? Kwargs are expensive to parse in functions that get called all the time, too.

I think my second proposal in the issue would work well (minus the render outline only thing, I think that’s unnecessary now)

font.outline_width (attribute, get/set with TTF_ outline)
font.outline_color (attribute, defaults to black)

Render with an outline:
font.render

narilee2006 · 2024-04-27T16:28:01Z

You can do the outline color by specifying it in the render() method with no modifications to the existing code.

I'm personally against the outline_color parameter as existing draw methods have the width parameter to determine their border size; if the width parameter is greater than 0 it won't fill the shape and only draw the borders.

https://pyga.me/docs/ref/draw.html

narilee2006 · 2024-04-27T16:50:01Z

Well then how do you do the color? Another kwarg? Kwargs are expensive to parse in functions that get called all the time, too.

@Starbuck5, sorry I meant the outline kwarg in the constructor of the Font object.

MyreMylar · 2024-04-28T13:48:45Z

I agree with @Starbuck that this should be implemented as a styling attribute to Font - similar to the current styling attributes bold, italic & underline. The current .render() function is already overlong with parameters like the 'antialias' parameter that definitely should have been an attribute defaulting to the most common usage (true).

Proposed test program:

import pygame
from pygame import Color, Font, QUIT

pygame.init()
window_surf = pygame.display.set_mode((400, 200))

impact_font = Font(filename="fonts/impact.ttf", size=64)

impact_font.outline_width = 2  # defaults to 0
impact_font.outline_color = Color("red")  # defaults to black

text_with_outline = impact_font.render(
    text="Hello World", antialias=True, color=Color("white")
)

running = True
while running:
    for event in pygame.event.get():
        if event.type == QUIT:
            running = False

    window_surf.fill(Color("black"))

    window_surf.blit(text_with_outline, (50, 50))

    pygame.display.update()

font.render can just check if outline_width is greater than 0 or not - and if so draw the outline before or after the main font rendering code - whichever looks better.

As an aside maybe we could make color and antialias into arguments with default values. We almost certainly would have already converted 'antialias' if it was on the other side of 'color' in the argument list. At least that way you could do:

font.render("Hello World", color=Color("white"))

Which isn't a typing saving over:

font.render("Hello World", True, Color("white"))

But probably clearer to read.

Might also open up:

font.default_color = Color("red")

hw_text_surf = font.render("Hello World in red")
other_text_surf = font.render("Other text, also in red")

MyreMylar

I think 80% of the time when people use this they are going to want to draw an outline around actual visible text.

A possible upgrade to font.render() that might be nice in concert with this is to allow it to set alpha pixels on the produced surface. I believe at the minute text surfaces come out of render in a slightly odd format that looks like normal 32 bit 'ARGB' - but is subtly not that. You can see this by doing:

text_surf  = font.render("Hello World", True, Color("white"))
# text_surf = text_surf.convert_alpha()
text_surf = text_surf.premul_alpha() 

running = True
while running:
    for event in pygame.event.get():
        if event.type == QUIT:
            running = False

    window_surf.fill(Color("grey"))

    window_surf.blit(text_surf, (50, 50), special_flags=BLEND_PREMULTIPLIED)

    pygame.display.update()

and seeing the difference after uncommenting the .convert_alpha() line. I expect at some point it was marginally cheaper to blit whatever, not-quite-full-transparency format comes out of font.render() but I doubt it still is. Anyway, if we could render semi- and full transparency text then you would get the benefits of outline only text without having to change the API.

narilee2006 · 2024-04-28T14:50:03Z

Okay... I'll get to work on this.

docs/reST/ref/font.rst

Starbuck5 · 2024-05-04T06:22:25Z

docs/reST/ref/font.rst

+      | :sl:`Gets or sets the font's outline color`
+      | :sg:`outline_size -> RGB`
+
+      Returns the color of the outline.


Ditto this doesn't just return it, it can be set here too. What does this default to?

Plus one of these attributes should have a small example of how to use this, with an image of what the output looks like.

Example as in the examples directory? Or an example as an example image in the documentation?

Example as in a code block in the docs showing how it works, with a picture of the rendered output.

src_c/font.c

Starbuck5 · 2024-05-04T06:25:58Z

src_c/font.c

    if (!PyUnicode_Check(text) && !PyBytes_Check(text) && text != Py_None) {
-        return RAISE_TEXT_TYPE_ERROR();
+        PyErr_Format(PyExc_TypeError, "text must be a unicode or bytes");
+        return 0;
    }

    if (wraplength < 0) {
-        return RAISE(PyExc_ValueError,
-                     "wraplength parameter must be positive");
+        PyErr_Format(PyExc_ValueError,
+                     "The wraplength parameter must be positive.");
+        return 0;
    }

    if (PyUnicode_Check(text)) {
        Py_ssize_t _size = -1;
        astring = PyUnicode_AsUTF8AndSize(text, &_size);
-        if (astring == NULL) { /* exception already set */
-            return NULL;
+        if (astring == NULL) {
+            return 0;  // exception already set.
        }
        if (strlen(astring) != (size_t)_size) {
-            return RAISE(PyExc_ValueError,
-                         "A null character was found in the text");
+            PyErr_Format(PyExc_ValueError,
+                         "A null character was found in the text.");
+            return 0;


Why do these changes exist?

Return type is no longer a PyObject *.

RAISE doesn't return a PyObject, it returns NULL. And NULL is 0, so this is the same code?

src_c/include/pygame_font.h

Starbuck5 · 2024-05-04T06:28:57Z

test/font_test.py

-            self.f = pygame_font.Font(None, 32)
+            self.f = pygame_font.Font(None, 96)


It's hard to see the outline when the font is small.

src_c/font.c

docs/reST/ref/font.rst

robertpfeiffer · 2024-05-04T09:04:55Z

src_c/font.c

+    }
+
+    SDL_Color foreg = {rgba[0], rgba[1], rgba[2], SDL_ALPHA_OPAQUE};
+    SDL_Color backg = {0, 0, 0, SDL_ALPHA_OPAQUE};


should we warn here when outline colour is the same as background or foreground?

I don't think that'll be necessary as printing content to the console is already a controversial topic.

pygame/pygame#1468

robertpfeiffer · 2024-05-04T09:07:29Z

src_c/font.c

+    static char *kwlist[] = {"text",    "antialias",  "color",
+                             "bgcolor", "wraplength", NULL};
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwds, "OpO|Oi", kwlist, &text,


having foreground colour and background colour as params, but outline colour as a property feels inconsistent. I would like to request some feedback from the maintainers on this. Maybe we should fix this now, maybe we need to make an issue for this after merge.

robertpfeiffer · 2024-05-04T09:09:01Z

src_c/font.c

+        return 0;  // exception already set
+    }
+
+    SDL_Color foreg = {rgba[0], rgba[1], rgba[2], SDL_ALPHA_OPAQUE};


I don't understand the API here, shouldn't the background be transparent by default? I know this matches the old behaviour, but this probably warrants a comment.

I learned from my professor that you cannot assign NULL to typedef structs. Correct me if she's wrong.

No, I meant I don't understand why the alpha isn't 0. It was like that before, but this part of the code is unclear to me.

It was originally SDL_ALPHA_OPAQUE before. https://github.com/pygame-community/pygame-ce/pull/2828/files#diff-75538c02a9bfe6511e8b72324ec5736b4d22aae6c5a99c52a83cef0dc70df300L575

src_c/font.c

robertpfeiffer · 2024-05-04T09:17:20Z

Overall, this is promising.

docs/reST/ref/font.rst

Starbuck5 · 2024-05-07T08:27:29Z

src_c/font.c

+}
+
+static int
+font_setter_outline_width(PyFontObject *self, PyObject *value, void *closure)


When I try to set a float it gives me this:

C:\Users\charl\Desktop\testr.py:8: DeprecationWarning: an integer is required (got type float). Implicit conversion to integers using __int__ is deprecated, and may be removed in a future version of Python. arial.outline_width = 1.5

This does not happen on my machine.

====================================================================== ERROR: test_set_outline_width_as_float (__main__.FontTypeTest.test_set_outline_width_as_float) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/nari/Documents/pygame-ce/test/font_test.py", line 392, in test_set_outline_width_as_float f.outline_width = 20.06 ^^^^^^^^^^^^^^^ TypeError: 'float' object cannot be interpreted as an integer ---------------------------------------------------------------------- Ran 72 tests in 13.958s

novialriptide · 2024-05-18T01:37:16Z

docs/reST/ref/font.rst

@@ -306,6 +306,30 @@ solves no longer exists, it will likely be removed in the future.

      .. ## Font.point_size ##

+   .. attribute:: outline_width


outline_radius is a better name if you're already going to mention radius in the docs description.

novialriptide · 2024-05-18T01:39:14Z

docs/reST/ref/font.rst

+   .. attribute:: outline_color
+
+      | :sl:`Gets or sets the font's outline color`
+      | :sg:`outline_width -> RGB`


You say (0, 0, 0, 255), but that's RGBA.

src_c/font.c

narilee2006 added 3 commits April 18, 2024 07:27

rm

6fbcb1a

Add outline & docs

3b15e7d

Add testing

bdb70ed

narilee2006 requested a review from a team as a code owner April 26, 2024 21:11

Remove README.md

fca9305

narilee2006 force-pushed the outlineee branch from 0861993 to 84b3b70 Compare April 27, 2024 04:00

Fix some bugs

a975224

narilee2006 force-pushed the outlineee branch from 5bb1ecd to a975224 Compare April 27, 2024 04:37

oddbookworm requested changes Apr 27, 2024

View reviewed changes

src_c/font.c Outdated Show resolved Hide resolved

buildconfig/stubs/pygame/font.pyi Outdated Show resolved Hide resolved

src_c/font.c Outdated Show resolved Hide resolved

test/font_test.py Outdated Show resolved Hide resolved

narilee2006 requested a review from oddbookworm April 27, 2024 07:33

oddbookworm reviewed Apr 27, 2024

View reviewed changes

test/font_test.py Outdated Show resolved Hide resolved

test/font_test.py Outdated Show resolved Hide resolved

Adjust code to Andrew Coffeey's reviews.

edcb35f

narilee2006 force-pushed the outlineee branch from 0f7db6e to edcb35f Compare April 27, 2024 12:11

narilee2006 requested a review from oddbookworm April 27, 2024 12:15

oddbookworm approved these changes Apr 27, 2024

View reviewed changes

Starbuck5 requested changes Apr 27, 2024

View reviewed changes

MyreMylar requested changes Apr 28, 2024

View reviewed changes

Re-implement outline with color and size properties.

0a617b9

narilee2006 force-pushed the outlineee branch from ff96f31 to 0a617b9 Compare April 28, 2024 22:32

Starbuck5 marked this pull request as draft April 29, 2024 05:01

Move font rendering implementation into a separate method

fc29c93

narilee2006 changed the title ~~Outlineee~~ Add outline option for rendering fonts Apr 29, 2024

narilee2006 added 2 commits April 30, 2024 16:05

Modify surface through reference

cd2142a

Creates a filled and outlined font

52c51ba

narilee2006 force-pushed the outlineee branch from 88485ea to 52c51ba Compare May 3, 2024 21:39

narilee2006 marked this pull request as ready for review May 3, 2024 21:40

narilee2006 requested review from Starbuck5, MyreMylar and oddbookworm May 3, 2024 21:42