make pdf phantom adjustments for new topo tool

[scd]

From Brendan: "the new topo tool the topo root element is a div which contains an img tag and an svg tag, where as in the old topo the svg element is the root".

[scd]

Fixed an issue:

dev.thecrag.com/topo/119361966

was throwing an error.

[scd]

Initial thoughts follow:

The old method in phantom: $(".phototopo").html() which gets the svg element which in turn contains the image.

The new method in phantom should combine the outer html of the following two selectors $(".phototopo > .canvas > img") and $(".phototopo > .canvas > svg"). Or alternatively just use the $(".phototopo > .canvas") selector then remove the div child element.

To be investigated:

1. Can the image just be concatenated in front of the svg, or does it have to be a child element as per old method. I imagine that the pdf stuff does not care.
2. The image from the new topo code does not seem to have a size on it, which confuses me a little. I am going to have to investigate further here because I think we need the image to have a target size in the pdf stuff.
3. The new topo svg has a structure which utilises the g container element. Can the PDF renderer handle this?. For the purposes of the PDF, do we even need the g container elements?

[scd]

Brendan, I get the following error when I run phantomjs

TypeError: 'undefined' is not a function (evaluating 'this.scrubStart.bind(this)')

Go to the phantom script directory and run:

phantomjs extract-topo-svg.js http://dev.thecrag.com/topo/119361966 255 140

Maybe pull from repo first to make sure we have got the same code.

Also it looks like you might be logging stuff to the console as there are other diagnostics coming out.

My guess is that this will be an easy fix for you, but somewhat time consuming for me to track down. Please advise if this is not the case.

[brendanheywood]

Ok this one worries me a little weird, .bind is a standard JS method added in ES5 but phantom doesn't implement it:

http://kangax.github.io/compat-table/es5/#Function.prototype.bind

I've added a polyfill for bind and now it works, but this feels like phantom needs an update

[brendanheywood]

Also some answer:

Can the image just be concatenated in front of the svg, or does it have to be a child element as per old method. I imagine that the pdf stuff does not care.

This is all around making the image responsive, the wrapper div around both svg and img is needed.

The image from the new topo code does not seem to have a size on it, which confuses me a little. I am going to have to investigate further here because I think we need the image to have a target size in the pdf stuff.

Again all around responsive stuff, the image is scaled to 100% of it's container div, the container div is sized however it needs to be depending on layout. Both the svg and img are scaled the same way. Adding an explicit size to the img will break things.

Google 'responsive intrinsic ratios' for details if you want. Short story is it means everything is responsive but also the document doesn't need to reflow as images load progressively so initial render is much faster.

As long as the PDF render has the correct styles in the markup and the correct wrapping html, and can interpret the css correctly it should all work, so if there are issues then look into that first, or point me at it.

The new topo svg has a structure which utilises the g container element. Can the PDF renderer handle this?. For the purposes of the PDF, do we even need the g container elements?

g is simply a group element in svg (like div in html) and is as old as svg itself, I can't see any reason why this wouldn't work. g is used extensively, I don't want to even consider removing it, it is one of the reasons why this new svg code is so fast and light. Also pretty sure raphael also used g as well, just not as much.

[brendanheywood]

Also you mentioned console.log stuff, there should be none of this. If you have it check that your topo code is current.

I thought we tested the recursive pull, but if you are still having issues add a git alias like this:

https://github.com/brendanheywood/scripts/blob/master/.gitconfig#L11-L14

and then you can type

git pull-all

[scd]

Thanks for the git alias -see issue #1624 for further discussion on git policy development.

I see that the polyfill for bind works on your dev. Where did you make that change, as it is not coming through in any of the repos. Can you please push? If it is in phototopo then this will be a good chance to test things.

It is good to get an understanding with what you are doing with the parent container div.canvas, especially in regards to sizing. This seems much more flexible, but I do like your confidence that it will just work for our pdf stuff. Anyway, if you can push the polyfill, I can test.

BTW, in the phantom script I have removed the div.actionable as this should not be required on a PDF. Please let me know if is.

[brendanheywood]

pushed

actionable is fine to be removed

[scd]

Brendan, your topo img tag should be properly closed for correct xhtml ( ie <img ... /> not <img ...>).

What about xmlns stuff. Is this something you should have in the topo svg output.

I will need your help with getting the topo svg working. I need to do a bit more investigation, but I think a couple of things may not be supported. My gut feeling here is that we are not close to resolving this and maybe we need a new PDF generator?

[brendanheywood]

Huh? xhtml died half a decade ago (thank god), we're using html5 not xhtml. In any case regardless of what the topo code spits out and how invalid it may be, you are exporting the dom from phantomjs which is it's internal representation of its stuff after it has parsed it, so changing the topo code won't make any difference.

So if the PDF thing needs xhtml then easiest thing is temp try to change the doctype of that template so when you export it, it comes out as xhtml (ok just tried this and it didn't work). So just regex it into shape, or worse case post process it with something like htmltidy to clean it up.

Can you be more specific with 'things not supported'? In terms of SVG I've used a subset of the svg that Raphael spit out so I don't think that should be a problem. I can imagine that some of the responsive css stuff which uses absolute positioning could barf the PDF thing but that's fairly easy to fix if that's the case.

Or if you're feeling ambitious you could look into ditching it completely and converting over to weasy #704 but that's opens up a few timing / release risks

[brendanheywood]

If the css is the issue, the easiest thing may be to post process it and throw away the responsive wrapper, cut out the img tag and convert it to an svg image inside the svg element like the old topo did.

<div class="canvas" style="padding-bottom: 54.6541417591802%">
<img class="fixedheightoversize" src="/image/topo/00001/fixedheightoversize/119361966.jpg" data-big="/image/topo/00001/fixedheightoversize/119361966.jpg">
<svg class="topooverlay" width="100%" height="100%" viewBox="0 0 1171 640">
...
</svg>
</div>

<svg class="topooverlay" width="{PDF width}" height="{PDF height}" viewBox="0 0 1171 640">
<image x="0" y="0" width="100%" height="100%" preserveAspectRatio="none" xlink:href="/image/topo/00001/fixedheightoversize/119361966.jpg" ></image>
...
</svg>

[scd]

Totally on the same page with this one. I just wanted to know if it was going to be me massaging the data or you changing your output. I am very happy to massage.

I ran out of time to further investigate, and will not get another chance until next week. I am pretty sure I am missing something obvious (but I have not investigated properly, so no need to do anthing yet). There are a few differences between your output and raphael's, most of which I don't think should make any difference. You use the following tags:

g - as discussed line - I really doubt this will be a problem use - ? pattern - ? defs - actually it is output in raphael, but is empty, you have it wrapping around some code

You also use 'transform' within one of the tags.

I will work out what is not ok. If it is a major problem then we can use phantom to render and throw in the image rather than svg.

I would love to rework the whole pdf as I think it is really important. However, not before this release - we have got to get it out the door. So we need to make sure it vaguely works as a temporary solution using our existing technology.

[scd]

Totally on the same page with this one. I just wanted to know if it was going to be me massaging the data or you changing your output. I am very happy to massage.

I ran out of time to further investigate, and will not get another chance until next week. I am pretty sure I am missing something obvious (but I have not investigated properly, so no need to do anthing yet). There are a few differences between your output and raphael's, most of which I don't think should make any difference. You use the following tags:

g - as discussed line - I really doubt this will be a problem use - ? pattern - ? defs - actually it is output in raphael, but is empty, you have it wrapping around some code

You also use 'transform' within one of the tags.

I will work out what is not ok. If it is a major problem then we can use phantom to render and throw in the image rather than svg.

I would love to rework the whole pdf as I think it is really important. However, not before this release - we have got to get it out the door. So we need to make sure it vaguely works as a temporary solution using our existing technology.

[scd]

I just noticed that the svg from the new photo-topo code is not autonomous, but requires the correct css to render correctly.

I can easily include the css in the pdf.css file, but I may have to work through some css compatibility issues with fop.

Are there any other places where we want svg?

[brendanheywood]

Yes the new topo is drastically smaller because style is in css, this also means lots of other wins in other ways. This shouldn't matter though as long as the portal.css file is included both when the topo is rendered, and then later when it is used in fop.

If you want you can link to make a new css file which imports jut the topo less file and link to that if you don't want the fill portal css if that clashes with other pdf css

[scd]

I have been drilling down through the issues with fop. It cannot handle multiple css files, which is not a problem because I just include into the pdf.

The phototopo css has some general editing css (and load icons) which fop cannot handle. So I remove these and at least I get the image to show. I think it would be worthwhile to split the phototopo.less into two files, one which has the minium information to show the phototopo svg elements, and another for the container management and editing.

Even paring this right back I still cannot get fop to apply the css to the svg elements. I could press on, but I think it fop sucks and will just now become a time waster. Note that pressing on may mean I reverse all the css back into the svg - yuk, unless we want a pure svg solution.

How do you feel about me writing a phantomscript to render the topo as a png of a specified size? This could be a stop gap solution until we rework the PDFs properly. At the moment we cannot release without the pdfs working, and as far as I can see it this is the only pragmatic solution. Any other ideas?

[brendanheywood]

Yes fop is dead, so lets just bake it into a raster and be done with it. In reality this should actually save size as the bulk of the size is the image which has to be there anyway.

The big caveat to this is that the raster has to be just high enough quality that the rendered lines actually look decent. The current sizes may be fine but need to check for legibility of lines and labels specially on bigger topos that get scaled down, like panoramic topos.

[scd]

Now this is almost done. I have converted PDFs to use png images of topos rather than svg. Please pull from repo, test and close.

It uses a new endpoint, for example:

http://dev.thecrag.com/topo/119361966/png?targetWidth=255&targetHeight=140

Which will be useful for other services as well. I imagine adam will need to port to this endpoint.

Note that we can go back to svg when we have a more robust pdf solution.

[brendanheywood]

I'm really happy with that, the quality is really good. For PDF's at least I'm not sure we actually need to go back to SVG. Only bug I can see if the default font is wrong, needs to be using Helvetica Neue.

[scd]

Do you mean the phantomjs script should set this? Or is this something that should be set in the css

[brendanheywood]

This is already set in the portal.css, so either it isn't loading, or perhaps it's working but phantom doesn't have the font installed or something deeper.

I just did a quick test and if I turn off the cropping of the image I get this:

So I think phantom, or the underlying os is missing a font. But at the very least I'd expect this to correctly fall back to arial or helvetica which should be installed?

[scd]

I have done a bit of investigation. It seems as though none of the above fonts were installed on Brendan's dev or prod. Arial was installed on simon's dev.

Helvetica Neue is a proprietary font which would cost us money to install. Helvetica seems problematic on Linux and I cannot find any good articles about installing it. Arial is easily installed using microsoft true type font package: apt-get install msttcorefonts which I have now done on both dev and prod. Please make sure this is ok and close the issue.

Do we need to raise an issue about Helvetica Neue not being consider a web safe font because some clients may not have it installed? Do we need to consider a different font-family choice?

[brendanheywood]

The css fonts cascade til it finds one that's is present, so it's all good with arial or normal helvetica as a base and neue as a progressive enhancement. I'd be happy with almost any simple sans serif font for the PDFs

See this for a good response by the bootstrap guys about the choice of neue https://github.com/twbs/bootstrap/issues/13823

Sent from my iPhone

On 19/09/2014, at 2:13 PM, Simon Dale notifications@github.com wrote:

I have done a bit of investigation. It seems as though none of the above fonts were installed on Brendan's dev or prod. Arial was installed on simon's dev.

Helvetica Neue is a proprietary font which would cost us money to install. Helvetica seems problematic on Linux and I cannot find any good articles about installing it. Arial is easily installed using microsoft true type font package: apt-get install msttcorefonts which I have now done on both dev and prod. Please make sure this is ok and close the issue.

Do we need to raise an issue about Helvetica Neue not being consider a web safe font because some clients may not have it installed? Do we need to consider a different font-family choice?

— Reply to this email directly or view it on GitHub https://github.com/theCrag/website/issues/1623#issuecomment-56133421.

[scd]

It defaults to Arial, so I think we can close this then now. Please re-open if we need to do any more work