rdkit / rdkit

The official sources for the RDKit library
BSD 3-Clause "New" or "Revised" License
2.59k stars 864 forks source link

New drawing code positions index number of primary amino group too high #3283

Closed apahl closed 4 years ago

apahl commented 4 years ago

Configuration:

Hi, I am just trying the new drawing code, which renders beautiful images of molecules, but I noticed the new drawing code seems to place index numbers in groups, e.g. the 2 of a primary amino group too high in the drawing:

from rdkit.Chem import AllChem as Chem
from rdkit.Chem import Draw
from rdkit.Chem.Draw import rdMolDraw2D
from rdkit.Chem import rdDepictor
rdDepictor.SetPreferCoordGen(True)
from rdkit.Chem.Draw import IPythonConsole
from IPython.display import SVG
import rdkit
print(rdkit.__version__)

l_threonine = Chem.MolFromSmiles('C[C@@H](O)[C@H](N)C(O)=O')

d2d = rdMolDraw2D.MolDraw2DSVG(300,280)
d2d.DrawMolecule(l_threonine)
d2d.FinishDrawing()
SVG(d2d.GetDrawingText())

rdkit_drawing

Expected: The number 2 should be positioned lower. The old drawing code does this correctly.

Many thanks for your help.

Kind regards,
Axel

greglandrum commented 4 years ago

Axel, I think that one is fixed. Can you please try updating to 2020.03.4 (available from conda-forge)?

apahl commented 4 years ago

Hi Greg, thanks a lot for coming back so quickly to this. In RDKit 2020.03.4 I get the same result:

rdkit_drawing

Kind regards,
Axel

greglandrum commented 4 years ago

Hmm, I don’t get that. Which OS are you using and which browser are you displaying the SVG in?

apahl commented 4 years ago

OS is Ubuntu 19.10, the Browser is Firefox 79.0b5 (64-bit).

Wow, it's indeed a Browser issue. I just opened the Notebook in Chromium and there the drawing is fine:

image

I do not know what to make of this, since I would like to continue using FF, but many thanks for your help.

greglandrum commented 4 years ago

We did notice problems with font handling in some SVG viewers. Since I don't think we're actually doing anything wrong in the code I'm not sure it's solvable without a bigger change. :-(

That bigger change is already in place in master: we've switched to using freetype and drawing the fonts ourselves. This results in better control over how the characters are placed and should almost certainly solve this problem. That will be in the 2020.09 release (and in the next RDKit nightly build).

apahl commented 4 years ago

Yes, rendering differences between viewers (and browsers for that matter) can be really annoying.

That's a great perspective then, thanks a lot.

RDKit is awesome!

Kind regards,
Axel

apahl commented 4 years ago

The document for the SVG 1.1 support in Firefox states that baseline-shift ist not working (section Text Module). It seems that SVG support in Firefox is literally not up to standard, which is disappointing.

DavidACosgrove commented 4 years ago

Sorry to be late seeing this one. This was definitely a feature of Firefox and one of the reasons I persuaded myself to take on using freetype to handle text in the drawings. The new code has freetype and non-freetype implementations for both Cairo and SVG, and the subscripts are correct in all of them in all the browsers I tried, on Linux and macOS. I wonder if we could persuade Greg that this is a bug fix and get it out earlier ;-) ?

Cheers, Dave

On Sun, Jul 12, 2020 at 1:23 PM Axel Pahl notifications@github.com wrote:

BTW, the document for the SVG 1.1 support in Firefox https://developer.mozilla.org/en-US/docs/Web/SVG/SVG_1.1_Support_in_Firefox states that baseline-shift ist not working (section Text Module. It seems that SVG support in Firefox is literally not up to standard, which is disappointing.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rdkit/rdkit/issues/3283#issuecomment-657214894, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACGF2FRYXVTO2Y3GK4NMMLTR3GTM3ANCNFSM4OWWL4JQ .

-- David Cosgrove Freelance computational chemistry and chemoinformatics developer http://cozchemix.co.uk

greglandrum commented 4 years ago

Unfortunately, that is definitely a new feature.

I will push a new beta build in the next few days and @apahl can maybe try that?