haskell-game / dear-imgui.hs

Haskell bindings to Dear ImGui, an immediate mode GUI toolkit
BSD 3-Clause "New" or "Revised" License
143 stars 31 forks source link

Extended font & glyph support #114

Closed chekoopa closed 2 years ago

chekoopa commented 3 years ago

There are many font-related functions to be ported into the library. So, I'd like to self-assign for that.

Roadmap:

Possible additions:

Basic support

Now font functions are situated in DearImGui.Fonts module (all glyph manipulations are expected to be here in the future). This module features the full-argument version of addFontFromFileTTF:

addFontFromFileTTF' :: MonadIO m 
  => FilePath -- ^ Font file path
  -> Float    -- ^ Font size in pixels
  -> Maybe Raw.FontConfig   -- ^ Configuration data
  -> Maybe Raw.GlyphRanges  -- ^ Glyph ranges to use
  -> m (Maybe Raw.Font) -- ^ Returns font handle, if added successfully

Font config support

It is not done on the first commit. First, we need to decide with interface for struct values and find out which values are used commonly and which are not, and do we even need getters or we can settle with setters only.

For example, merge mode is much desired to support range mixing and icon fonts, like here (excerpt from ImGui docs):

// Load a first font
ImFont* font = io.Fonts->AddFontDefault();

static const ImWchar icons_ranges[] = { 0xf000, 0xf3ff, 0 }; 
ImFontConfig config;
config.MergeMode = true;
io.Fonts->AddFontFromFileTTF("DroidSans.ttf", 18.0f, &config, io.Fonts->GetGlyphRangesJapanese()); // Merge into first font
io.Fonts->AddFontFromFileTTF("fontawesome-webfont.ttf", 18.0f, &config, icons_ranges);             // Merge into first font
io.Fonts->Build();

Possible Haskell alternative is:

addFontDefault
let iconsRanges = Just (rangesFromPairs [(0xf000, 0xf3ff)])
config <- newConfig
setMergeMode true config
addFontFromFileTTF "DroidSans.ttf" 18.0 (Just config) (Just getGlyphRangesJapanese)
addFontFromFileTTF "fontawesome-webfont.ttf" 18.0 (Just config) iconsRanges
buildAtlas

No way we could make FontConfig modifications pure, because it is definitely stateful (as being changed through font adding).

Though, I'm not sure about rangesFromPairs, because it would assume low-level memory operations (with malloc), which is unsafe, so we got to settle with...

Range builder support

There is a problem with data transfer. Range builder puts its data into some vector, and we need to extract its data to utilize it in the function. Seems there are possible caveats with early destruction and null pointers, but I hope it's not.

So, we would come from this:

ImVector<ImWchar> ranges;
ImFontGlyphRangesBuilder builder;
builder.AddText("Hello world");                        // Add a string (here "Hello world" contains 7 unique characters)
builder.AddChar(0x7262);                               // Add a specific character
builder.AddRanges(io.Fonts->GetGlyphRangesJapanese()); // Add one of the default ranges
builder.BuildRanges(&ranges);                          // Build the final result (ordered ranges with all the unique characters submitted)

io.Fonts->AddFontFromFileTTF("myfontfile.ttf", size_in_pixels, NULL, ranges.Data);
io.Fonts->Build();                                     // Build the atlas while 'ranges' is still in scope and not deleted.

To this:

builder <- newBuilder 
addText "Hello world" builder -- String or Text?
addChar 0x7262 builder -- we still need figure out how to send literal as a ImChar
addRanges getGlyphRangesJapanese builder
ranges <- buildRanges builder -- creates vector, builds into it and returns the handle

addFontFromFileTTF "myfontfile.ttf" 14 Nothing (Just $ getRangesData ranges);
buildAtlas

Something tells me we even can implement these transformations as pure functions:

let builder = addRanges getGlyphRangesJapanese $ addChar 0x7262 $ addText "Hello world" $ newBuilder 
    ranges = getRangesData $ buildRanges builder -- creates vector, builds into it and returns the handle
    -- It asks to get rid of 'getRangesData' and make 'buildRanges' output straight 'GlyphRanges'
    -- Though, I don't know how would it look in terms of memory safety.

addFontFromFileTTF "myfontfile.ttf" 14 Nothing (Just ranges);
buildAtlas

UPD: But nevermind, it requires copying, which is a bad idea here. I'd even like implement the builder as a monad, using a block with bracket, which would cleanup the builder after outputting the ranges in a vector.

chekoopa commented 3 years ago

Currently, I have a problem with ImWchar type. I've wanted to express it as Word16, but the usage of IMGUI_USE_WCHAR32 would enable using emoji, shapes, ancient languages etc. So, we should somehow switch the type in Haskell.

Moreover, I even can't build a C code using it. It is marshalled in the ImGui context, but it doesn't compute.

data ImWchar
-- neither empty declaration nor `Word16` builds
-- compiles with `type ImWchar = Word16`, but still needs 32-bit branch

------------------

imguiContext = mempty
  { ctxTypesTable = enumerationsTypesTable <>
    Map.fromList
      [ ( TypeName "ImVec2", [t| ImVec2 |] )
      -- ...
      , ( TypeName "ImWchar", [t| ImWchar |] )
      -- ...
      ]
  }

------------------

-- | Add character
addChar :: MonadIO m => GlyphRangesBuilder -> ImWchar -> m ()
addChar (GlyphRangesBuilder builder) wChar = liftIO do
  [C.block|
    void {
      $(ImFontGlyphRangesBuilder* builder)->AddChar($(ImWchar wChar));
    }
  |]

------------------

src/DearImGui/Raw/GlyphRangesBuilder.hs:67:12: error:
    • Unacceptable argument type in foreign declaration:
        ‘ImWchar’ cannot be marshalled in a foreign call
    • When checking declaration:
        foreign import ccall safe "inline_c_DearImGui_Raw_GlyphRangesBuilder_2" inline_c_ffi_6989586621679095017
          :: Ptr ImFontGlyphRangesBuilder -> ImWchar -> IO ()
   |
67 |   [C.block|
   |       

UPD: I've managed to build it with type ImWchar = Word16. Yet we need to process IMGUI_USE_WCHAR32 somehow.

dpwiz commented 3 years ago

Builder can be split into instructions and execution. With instructions being a monoid (semigroup?), executing a builder could be done with a single procedure, without externally-visible pointers and other details.

setupFonts = do
  FontAtlas.clear
  helloRanges <- FontRanges.build
    [ FontRanges.text "Hello world"
    , FontRanges.char 'Ꙑ'
    , FontRanges.ithkuil4  -- preconfigured range combinators
    ]
  FontAtlas.addFileTTF "myfontfile.ttf" 14 Nothing helloRanges
  FontAtlas.build

FontAtlas setup can be purified too:

setupFonts = FontAtlas.rebuild
  [ FontAtlas.FileTTF "comic-sans-mono.ttf" 14 Nothing -- defunctionalized addFontFromFileTTF
    [ FontRanges.text "Hello world"
    , FontRanges.char 'Ꙑ'
    , FontRanges.ithkuil4
    ]
  , FontAtlas.hellveticaSerif 32 -- preconfigured font combinators
  , FontAtlas.Default
  ]
chekoopa commented 3 years ago

Builder can be split into instructions and execution. With instructions being a monoid (semigroup?), executing a builder could be done with a single procedure, without externally-visible pointers and other details.

Thought about that, too. Good choice for high-level implementation. And instructions don't need to be monoid, as even in the example they are treated as some list elements.

FontAtlas setup can be purified too

Also an interesting idea, however not sure about running that once a frame.

Most of my concerns are about low-level detail at the moment. Like, 32-bit char wideness and font config accessors. But high-level implementation is really going to look like you've described.

dpwiz commented 3 years ago

however not sure about running that once a frame.

Font atlas is persistent. Rebuilding it each frame is of course a bad idea.

dpwiz commented 3 years ago

wchar32 can be addressed separately in #115

chekoopa commented 3 years ago

Builder can be split into instructions and execution.

Actually, it bothers me that GlyphRangesBuilder builds into an ImVector, and we have to access ImVector::Data to utilize ranges for font adding procedures. So we likely have to give up GlyphRanges parameter type for ranges builder lists.

Also, representing and marshalling ImVector<ImWchar> is also a little low-level issue:

-- at imguiContext -> ctxTypesTable
( Template "ImVector" [TypeName "ImWchar"], [t| ImVector<ImWchar> |] )

---------------

src/DearImGui/Context.hs:43:73: error:
    Operator applied to too few arguments: >
   |
43 |       , ( Template "ImVector" [TypeName "ImWchar"], [t| ImVector<ImWchar> |] )
   |                                                                         ^
dpwiz commented 3 years ago

Wouldn't an opaque pointer be enough? I don't think we need to open up its data at the Haskell level.

chekoopa commented 3 years ago

Wouldn't an opaque pointer be enough? I don't think we need to open up its data at the Haskell level.

Like in the snippet below? FYI, I'm not much experienced in C/C++, so feel free to correct me if I'm wrong. But it builds.

I really don't want to return ImWchar* straight from a vector, but user would definitely use ranges <- fromRangesVector <$> buildRanges builder, leading to loss of vector pointer and memory leak... Maybe, I'm just overthinking. I know, we'll somehow address this issue in high-level implementation (withRangesBuilder buildInstructionsHere $ \ranges -> ...), but it still worries me.

newtype GlyphRangesVector = GlyphRangesVector (Ptr ())

-- |  Output new ranges
buildRanges :: MonadIO m => GlyphRangesBuilder -> m (GlyphRangesVector)
buildRanges (GlyphRangesBuilder builder) = liftIO do
  GlyphRangesVector <$> [C.block|
    void* {
      ImVector<ImWchar>* ranges = IM_NEW(ImVector<ImWchar>);
      $(ImFontGlyphRangesBuilder* builder)->BuildRanges(ranges);
      return ranges;
    }
  |]

fromRangeVector :: GlyphRangesVector -> GlyphRanges
fromRangeVector (GlyphRangesVector vecPtr) = unsafePerformIO do
  GlyphRanges <$> [C.block| 
    ImWchar* { 
      return ((ImVector<ImWchar>*) $(void* vecPtr))->Data;
    } 
  |]

-- So, as I have to use a vector pointer, it definitely asks for a destructor.

destroyRangesVector :: MonadIO m => GlyphRangesVector -> m ()
destroyRangesVector (GlyphRangesVector vecPtr) = liftIO do
  [C.block|
    void {
      IM_DELETE(((ImVector<ImWchar>*) $(void* vecPtr)));
    }
  |]
chekoopa commented 3 years ago

It works, but it really needs some proper work on resource destruction...

If low-level implementation is okay, I'll continue with high-level implementation. FontConfig is still in question. We may use similar syntax with instruction list, but it will definitely be bulky. Wouldn't use monoid record though, as it would likely force us to synchronize defaults with the library.

-- in main function of fonts example
builder <- GRB.new

GRB.addRanges builder getGlyphRangesDefault
liftIO $ withCString "Привет" \txt -> GRB.addText builder txt
rangesVec <- GRB.buildRanges builder
let ranges = GRB.fromRangesVector rangesVec

addFontFromFileTTF' "./imgui/misc/fonts/DroidSans.ttf" 12 
                               Nothing (Just ranges) >>= \case
                               -- Nothing (Just getGlyphRangesCyrillic) >>= \case
  Nothing -> error "couldn't load DroidSans.ttf"
  Just font -> return font

-- ... skipping default and noto fonts addition ...

buildFontAtlas  -- now it is necessary as hell

-- resource destruction comes only after the building
GRB.destroyRangesVector rangesVec
GRB.destroy builder

Result is as expected:

2021-11-15_10 42 17_450x466_scrot

chekoopa commented 3 years ago

FontAtlas setup can be purified too

Actually, there is a problem with that solution: we need to return font handles for setting fonts in the main loop (it's implemented already with DearImGui.Fonts.withFont). We could return some map or list with corresponding fonts here, though it would require additional job with Maybe unpacking (even though font loading failures just crash with assert ATM).

I'd even like to craft a very unsafe contraption with null pointer, like the code below.

I think, I'll settle on monoid-like config interface and craft high-level API like you've suggested and adjust it according to your replies.

data Font = Font
  { font_settings :: FontSettings 
  , font_pointer :: Ptr ImFont
  }

defaultFont :: Font
defaultFont = Font Default nullPtr

ttfFontFromFile :: FilePath -> Float -> Font
ttfFontFromFile path size defaultFont = Font (FileTTF path size) nullPtr

initFont :: MonadIO m => Font -> m ()
initFont (Font Default fontPtr) = 
  [C.block|
    void {
      $(ImFont* fontPtr) = GetIO().Fonts->AddFontDefault();
    }
  |]
-- and etc

rebuild :: MonadIO m => [ Font ] -> m ()
rebuild fonts = do
  clearFontAtlas
  mapM initFont fonts
  -- TODO: we could try-catching initFont errors here and return instead of ()
  -- TODO: resource destruction for range builders and, possibly, font configs
  buildFontAtlas

withFont :: MonadUnliftIO m => Raw.Font -> m a -> m a
withFont font = do
  -- TODO: assert 'font_pointer font' is not nullPtr
  bracket_ (Raw.pushFont font) Raw.popFont
dpwiz commented 3 years ago

Actually, there is a problem with that solution: we need to return font handles for setting fonts in the main loop

FontAtlas.rebuild can traverse a collection, replacing a t Settings with t Handle. This way you can have no fonts (()), one font (Just ...), a list of fonts, a Map of fonts, and an even no-holds-barred ADT of fonts.

chekoopa commented 3 years ago

FontAtlas.rebuild can traverse a collection, replacing a t Settings with t Handle.

Like this? Sounds very promising.

rebuild :: (Traversable t, MonadIO m) => t Font -> m (t Handle)
dpwiz commented 3 years ago

Like this? Sounds very promising.

Yep. I tend to use this a lot in my games. A collection is a guaranteed functor, allowing some nice tricks by having different wrappers. It is even nicer with Applicative instance.

chekoopa commented 2 years ago

It is even nicer with Applicative instance.

optparse-applicative is a very nice example of approach you've seemingly hint to. Moreover, Alternative sounds handy in this context (like, if we can't load a font, we could take another).

But I'll stick with Traversable atm. Because the design slowly steers to overthinking (we can implement applicative interface later). And also ImGui crashes with assert, once font isn't found (thus, I'm not sure we could catch it). Though, I'm going to implement Semigroup/Monoid on font config and ranges builder parts of this interface.

dpwiz commented 2 years ago

And also ImGui crashes with assert, once font isn't found (thus, I'm not sure we could catch it).

Would doing a doesFileExist before adding font help getting a nicer error?

chekoopa commented 2 years ago

Would doing a doesFileExist before adding font help getting a nicer error?

It would. But I'm afraid that there are more asserts in library to consider. We could disable them, actually (#115), and loading failures would produce already-processed null pointer. I throw IOError with fail on such case.

And also you've reminded me that resource allocation and destruction should be wrapped with bracket or even managed...

UPD: Managed-based implementation is complicated, forcing me to get rid of low-level functions, as they would be converted to Managed too, which would force their usage through wrapping, like with or using. However, I don't feel like it's something bad, so, I'm sticking to it.

dpwiz commented 2 years ago

Managed is fine for examples.

chekoopa commented 2 years ago

Managed is fine for examples.

Yes, but I see its usage in the DearImGui module, so why not bracket-ing some of allocations here and there?

Actually, I'm seemingly done with implementation, so it's wide open to review and propose some more changes, or just merge into. I'll clone additions list here to consider and discuss:

dpwiz commented 2 years ago

I'm about to send a PR to your PR with a bunch of stuff I'm too lazy to type out in prose :smile:

chekoopa commented 2 years ago

I'm about to send a PR to your PR with a bunch of stuff I'm too lazy to type out in prose :smile:

Sure, I'd like to look and examine that (never stop learning :eyes:). Just consider I've pushed an auto-derivation in the example, but haven't touched elsewhere.

Also, my SDL library instance stopped working (I use NixOS 21.11) and crashes with "Invalid window", so I had to update haskell.nix pin and wait for an hour to build and test this example (haven't pushed new pins, though). Can't really investigate this right now, and it also alerts me about the state of Nixpkgs library. :thinking:

chekoopa commented 2 years ago

It bothers me that CI fails with this:

error: experimental Nix feature 'nix-command' is disabled; use '--extra-experimental-features nix-command' to override
dpwiz commented 2 years ago
examples/vulkan/Main.hs:141:3: error:

This is "fixed" in my PR.

chekoopa commented 2 years ago

PR merged, seems ready to roll.