canismarko / dungeon-sheets

A tool to create character sheets and GM session notes for Dungeons and Dragons fifth edition (D&D 5e).
https://dungeon-sheets.readthedocs.io/en/latest/
GNU General Public License v3.0
161 stars 66 forks source link

Portrait slightly off-center in MSavage latex template #157

Closed PJBrs closed 6 months ago

PJBrs commented 8 months ago

Since https://github.com/canismarko/dungeon-sheets/commit/e0061827373741efc2f8f516c2c2f079b817fa31 the portrait position in the MSavage latex template is a bit off-center. I tried to find the "right" location myself, and ended up with the following definition in a character file:

images = [("image.jpeg", 1, 114.5, 555, 165, 215)]

I do think that the previous latex command worked very well too:

\includegraphics[width=5.75cm]{" + portrait_file + "}

@Valentin-Metz - perhaps you have an idea for this one? I tried a bit myself, with

if use_tex_template

but I didn't quite know which way to go...

Valentin-Metz commented 8 months ago

What image did you use for this? - Maybe we can compile a test suite of a few images with varying aspect ratio. Currently we are not using LaTex to insert the images (we are instead stamping the image over an arbitrary position in the pdf).

PJBrs commented 8 months ago

I don't think it's the aspect ratio, it really is slightly off-center. I think the appearance box is located sligthtly to the left on the MSavage sheet compared to the original one.

Anyway, I made some portraits with artbreeder which I just, I'll just upload one!

drompf

Valentin-Metz commented 8 months ago

Can you give me the full character.py file you used for this one? I'll try to reproduce it locally.

Valentin-Metz commented 8 months ago

Ah, I just realized that MSavage is a specific tex template. I haven't tested the image generation for these. All we'd need to fix this is an if statement in character.py at line 195 to check for what type of sheet we are generating.

PJBrs commented 8 months ago

... that's what I thought too! Although you could also opt to let latex do the heavy lifting for the MSavage template... Imagine, for instance, compiling for a4paper instead of letterpaper, which would require a different positioning again.

Valentin-Metz commented 8 months ago

That'd also be a good idea (at least for the character image + symbol). The change would probably be slightly more complex however, as we'd need to get the path of the image to the latex compiler. Just stamping it at a different location is probably easier. Can we somehow find out in character.py whether we are using MSavage?

PJBrs commented 8 months ago

It's a commandline option ( -T) to make-sheets. It sets use_tex_template. I don't know, however, to call that variable from within character.py...

canismarko commented 8 months ago

Thanks for working on this, all. I think in general it's better to keep the LaTeX-specific things in make_sheets.py, that way character.py can be limited to describing the characters only. It would get tricky if you then used --epub instead of generating a LaTeX sheet. I will look closer when I have a minute and see if I can find the place in make_sheets where this would be possible.

PJBrs commented 8 months ago

This is one way to fix it:

diff --git a/dungeonsheets/make_sheets.py b/dungeonsheets/make_sheets.py
index d4dd565..ee28d26 100644
--- a/dungeonsheets/make_sheets.py
+++ b/dungeonsheets/make_sheets.py
@@ -506,8 +506,18 @@ def msavage_sheet(character, basename, debug=False):
     """Another adaption. All changes can be easily included as options
     in the orignal functions, though."""

+    # Load image file if present
+    portrait_command = ""
+    if character.portrait:
+        for image in character.images:
+            if re.search(r"" + character.portrait, str(image)):
+                character.images.remove(image)
+                break
+        portrait_command = r"\includegraphics[width=5.75cm]{" + character.portrait + "}"
+
+
     tex = jinja_env.get_template("MSavage_template.tex").render(
-        char=character, portrait=""
+        char=character, portrait=portrait_command
     )
     latex.create_latex_pdf(
         tex,
PJBrs commented 7 months ago

Hmm... There seems to be another issue with this image-handling as well, it appears that the list of pdfbookmarks goes missing...

canismarko commented 7 months ago

So your fix works but breaks the pdf bookmarks? I'm not sure what to make of that.

PJBrs commented 7 months ago

I'm not expressing myself clearly. What I meant to say - I think that the new image handling codepaths by @Valentin-Metz removes the pdfbookmarks. I just checked out master and compiled the examples. You'll note in the image below how bard2.pdf has a contents list (i.e., the pdfbookmarks) in the left-hand pane, in addition to the thumbnails. Bard1.pdf, which includes images, only has thumbnails, it doesn't have the contents anymore. Since my patch above restores the old codepath for the MSavage latex templates, it also keeps the contents intact.

image

Valentin-Metz commented 7 months ago

I've opened a pull request that fixes the bookmarks being discarded when the images are merged in (https://github.com/canismarko/dungeon-sheets/pull/161). (This not change the interaction with the MSavage latex template in any way) The bookmarks disappearing was an oversight from me and has nothing to do with the changes from @PJBrs.

PJBrs commented 7 months ago

@Valentin-Metz Thanks for the patch, it works!

PJBrs commented 7 months ago

I added the following to my local branch as well, to fix symbol position:

+    # Move symbol image a bit left, if applicable
+    if character.symbol:
+        for image in character.images:
+            if re.search(r"" + character.symbol, str(image)):
+                character.images.remove(image)
+                character.images = [(character.symbol, 1, 488, 564, 145, 112)] + character.images
+                break

I could't do this with the latex template, because it doesn't have a \parbox that positions the symbol correctly. And anyway, that location wouldn't need both text and image anyway.

I'll make a new PR with some bug fixes after my open ones have been pulled or closed.