Mark parseGlyfGlyph prototype as gcsafe

[ghost]

This causes problems for procs that call fillText and need to be gcsafe.

[treeform]

Could you provide a short example of where this happens? I am not sure what the problem is.

[ghost]

Sorry, the proc that needed to be marked was further down in opentype.nim.

The problem is that when a proc is declared without a body the compiler will assume that it is not gcsafe, and any proc that references that proc declaration becomes implicitly un-gcsafe as well.

[treeform]

What happens when you move gcsafe not further down, but up into your own code?

[ghost]

If I put gcsafe high up in my code things work, but I'm rendering in nested callbacks, so the trace back to the original function takes some effort to follow. And if I mark something high up as gcsafe to silience an error then I might miss an actually gcunsafe proc later.

[treeform]

So you have threads and callbacks, which requires gc safe? This feels like a nim issue not a pixie issue?

Eventually all pixie methods would require gc safe, no? I don't think it's workable to mark only the methods you use as gc safe? Or do you think only some key methods need the gc safe tag for things to work?

We need a simple test case of this before we merge. And a better explanation. I don't get why you need gc safe at all and can't have it in your user code.

[ghost]

https://nim-lang.org/docs/manual.html#effect-system-gc-safety-effect

Doesn't compile:

import std/asyncdispatch
import pixie

proc main() {.gcsafe.} =
  let image = newImage(200, 200)
  image.fill(rgba(255, 255, 255, 255))
  
  var font = readFont("examples/data/Roboto-Regular_1.ttf")
  font.size = 20
  
  let text = "Hello world"
  
  image.fillText(font.typeset(text, vec2(180, 180)), translate(vec2(10, 10)))
  image.writeFile("examples/text.png")

waitFor main()

Gcsafe is necessary for safe callbacks, and using callbacks is unavoidable in some cases.

[treeform]

I think you have a bug in your code, waitFor needs an async proc not a gcsafe proc, it throws some strange error instead. I think the correct code is (compiles on my machine ™️):

import std/asyncdispatch
import pixie

proc main() {.async.} =
  let image = newImage(200, 200)
  image.fill(rgba(255, 255, 255, 255))

  var font = readFont("/p/pixie/examples/data/Roboto-Regular_1.ttf")
  font.size = 20

  let text = "Hello world"

  image.fillText(font.typeset(text, vec2(180, 180)), translate(vec2(10, 10)))
  image.writeFile("text.png")

waitFor main()

[treeform]

I also don't get why it correctly infers that that:

• fillText is gc safe
• textUber is gc safe
• getGlyphPath is gc safe
• parseGlyph is gc safe
• parseGlyfGlyph is for some reason not?

What is that reason?

[treeform]

It appears that Nim can't GC safe recursive proc calls.

parseGlyfGlyph calls parseCompositeGlyph which can call parseGlyfGlyph

It gets confused and marks whole tree not gc safe even though it is?

[treeform]

I have filed a bug with Nim: nim-lang/Nim#19339

[treeform]

Hey @ehmry, you where right! It looks like we need to add gc safe to all forward declaration.