Asymmetric offsets for double bonds

[arose]

To draw extra cylinders/lines inside a ring

[fredludlow]

I've got an initial commit for this here: fredludlow/ngl@11faf5d3295ca3deb314803f87e5d180079cf313

Looks nice for lines, looks okay for licorice (the cylinders are uncapped - might need to place extra spheres at the ends to round them off?) and looks a bit sketchy for ball+stick mode.

[arose]

Looks great!

Does the amount of "shortening" need to be adjusted for different values of bondSpacing?

the cylinders are uncapped - might need to place extra spheres at the ends to round them off?

Yeah would be nice, but can be done in the future. Either with extra spheres or rounded caps (https://github.com/arose/ngl/issues/187). In both cases it requires some refactoring as the placement of the spheres and the "identity" of the shifted bonds is only known within the getBondData method.

looks a bit sketchy for ball+stick mode.

I guess that is okay. One could change the aspectRatio param for bigger spheres.

[fredludlow]

Thanks!

Does the amount of "shortening" need to be adjusted for different values of bondSpacing?

Probably - actually the parameters need a bit of a think through (I'm currently abusing the bondSpacing parameter because I just wanted a way to play with relative radii in the UI easily without wiring up another parameter).

I think for Balll+Stick the current (symmetric) rendering looks better anyway (it looks a bit more like the plastic molecular modelling kits you can buy) so maybe for that mode at least the current (two smaller symmetric cylinder) mode should be default.

I wasn't sure what the most intuitive set of parameters would be to expose the various options, multipleBond could take values null, symmetric, offset? Or there could be a separate param for offset/asymm? Then the parameters you need internally are a radius scaling factor, a shortening factor and an offset, which of those are independent and exposed to the user and which are calculated from others I don't really have a strong opinion on as long as the defaults look good. Do you have a preference?

[arose]

multipleBond could take values null, symmetric, offset

I like this approach

Do you have a preference?

not really, just not too many :-)

[fredludlow]

Okay, nearly there I think. What I've gone for in terms of parameters is:

• multipleBond is a select with 'off', 'symmetric', 'offset'
• There are two parameters, bondScale and bondSpacing:
  • bondScale - ranges from 0-1.0 - relative radius of the second bond for double bonds and (in the offset case) triple bonds. In the symmetric case there's another adjustment that takes bond order into account. Default of 0.4 seems okay
  • bondSpacing - In symmetric mode a value of 1 means the outside of the cylinders will be placed at the standard radius distance: This can vary between 0.5 and 2.0 to push the cylinders in and out In offset mode a value of 1 corresponds to the center of the offset cylinder being 2*radius offset:
• Bond shortening is calculated according to offset but there's a hard limit at 80% of the original bond length (otherwise you end up with weird effects with unusual combinations of other parameters).

When loading a PDB through the webapp menu the default representation for het groups is ball+stick with scale=2.0, I've added values for bondScale and bondSpacing so that if the user subsequently selects either of the multiple-bond modes it looks a bit better.

I'm afraid I've ended up with a fairly monolithic PR with both the ring stuff and this sorry.

[arose]

This is really great!

One little thing regarding bond shortening. Would look a bit better to shorten only one side of the bond for e.g. carbonyls?

[fredludlow]

Yes - I guess a quick "is it terminal" check would do it. What's the cleanest way to deduce this inside the Structure.getBondData method? (residue type and bondgraph, atom/bond proxy and bondHash, or something else...?)

Even better would be to be aware of the geometry and adjust cutoff accordingly (linear alkynes also look a bit funny at the moment) but that sounds like another level of perception of the molecule

[arose]

Yes - I guess a quick "is it terminal" check would do it. What's the cleanest way to deduce this inside the Structure.getBondData method? (residue type and bondgraph, atom/bond proxy and bondHash, or something else...?)

var isTerminal = bondHash.countArray[ atomIndex ] === 1;

[fredludlow]

Hmm, actually if you have different shortenings on both sides: fredludlow/ngl@39ce713 you end up with with color-changes not quite lining up:

I'm inclined to leave this as is for now, what do you think?

[arose]

... color-changes not quite lining up:

yeah fixing that will cause some headaches, not sure if it is worth it, I would say no. What do you think.

[fredludlow]

Agreed, I'll close this for now. Oops, can't close it, you're the owner!