Skip to content

Commit

Permalink
Merge pull request #1015 from geoadmin/bugfix-PB-818-font-and-image-s…
Browse files Browse the repository at this point in the history
…ize-dont-match-in-print

PB-818: Ensuring scales are respected on print - #minor
  • Loading branch information
pakb committed Sep 11, 2024
2 parents 5ad5753 + f4e125c commit 7bd41b0
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 60 deletions.
19 changes: 19 additions & 0 deletions src/api/__tests__/print.api.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { expect } from 'chai'
import { describe, it } from 'vitest'

import { PrintLayout, PrintLayoutAttribute } from '@/api/print.api.js'
import { PRINT_DPI_COMPENSATION } from '@/config/print.config'
import { adjustWidth } from '@/utils/styleUtils'

describe('Print API unit tests', () => {
describe('PrintLayoutAttribute tests', () => {
Expand Down Expand Up @@ -61,5 +63,22 @@ describe('Print API unit tests', () => {
)
expect(scalesInMapAttr.scales).to.eql(scales)
})
it('calculate the width correctly with the "adjustWidth" function', () => {
// invalid values should return 0
expect(adjustWidth(100, 'invalid value')).to.eql(0)
expect(adjustWidth(100, null)).to.eql(0)
expect(adjustWidth(100, undefined)).to.eql(0)
expect(adjustWidth(null, 254)).to.eql(0)
expect(adjustWidth(undefined, 254)).to.eql(0)
expect(adjustWidth('invalid value', 254)).to.eql(0)
// the dpi parameter should be a positive number
expect(adjustWidth(100, 0)).to.eql(0)
expect(adjustWidth(100, -15)).to.eql(0)
// checking with a slight margin for float rounding errors.
expect(adjustWidth(100, 254)).to.be.closeTo(
(100 * PRINT_DPI_COMPENSATION) / 254,
1 / 254
)
})
})
})
31 changes: 25 additions & 6 deletions src/api/print.api.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,18 @@ class GeoAdminCustomizer extends BaseCustomizer {
symbolizer.strokeWidth = adjustWidth(symbolizer.strokeWidth, this.printResolution)
symbolizer.haloRadius = adjustWidth(symbolizer.haloRadius, this.printResolution)
symbolizer.conflictResolution = false
symbolizer.fontSize =
Math.ceil(2 * adjustWidth(parseInt(symbolizer.fontSize), this.printResolution)) + 'px'
// we try to adapt the font size and offsets to have roughly the same
// scales on print than on the viewer.
try {
symbolizer.labelYOffset = adjustWidth(symbolizer.labelYOffset, this.printResolution)
symbolizer.labelXOffset = adjustWidth(symbolizer.labelXOffset, this.printResolution)
symbolizer.fontSize = `${adjustWidth(
parseInt(symbolizer.fontSize) * text.getScale(),
this.printResolution
)}px`
} catch (error) {
// Keep the font family as it is
}
}

/**
Expand Down Expand Up @@ -114,12 +124,23 @@ class GeoAdminCustomizer extends BaseCustomizer {
}

if (anchor) {
symbolizer.graphicXOffset = adjustWidth(-anchor[0] * scale, this.printResolution)
symbolizer.graphicYOffset = adjustWidth(-anchor[1] * scale, this.printResolution)
symbolizer.graphicXOffset = symbolizer.graphicXOffset
? adjustWidth(
(size[0] / 2 - anchor[0] + symbolizer.graphicXOffset) * scale,
this.printResolution
)
: 0
// don't ask why it works, but that's the best I could do.

symbolizer.graphicYOffset = symbolizer.graphicYOffset
? (symbolizer.graphicYOffset = adjustWidth(-size[1], this.printResolution))
: 0
}
if (size) {
symbolizer.graphicWidth = adjustWidth(size[0] * scale, this.printResolution)
}
symbolizer.graphicXOffset = symbolizer.graphicXOffset ?? 0
symbolizer.graphicYOffset = symbolizer.graphicYOffset ?? 0

if (symbolizer.fillOpacity === 0.0 && symbolizer.fillColor === '#ff0000') {
// Handling the case where we need to print a circle in the end of measurement lines
Expand Down Expand Up @@ -325,9 +346,7 @@ async function transformOlMapToPrintParams(olMap, config) {
if (!dpi) {
throw new PrintError('Missing DPI for printing')
}

const customizer = new GeoAdminCustomizer(excludedLayerIDs, dpi)

const attributionsOneLine = attributions.length > 0 ? ${attributions.join(', ')}` : ''

try {
Expand Down
3 changes: 3 additions & 0 deletions src/config/print.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// In the old mapviewer, a magic number (90) was set to make some compensation between the print and the viewer,
// to keep the scale between the two services. In the current implementation, 144 seems to be giving the best results.
export const PRINT_DPI_COMPENSATION = 144
10 changes: 5 additions & 5 deletions src/utils/styleUtils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Circle, Fill, Stroke, Style } from 'ol/style'
import CircleStyle from 'ol/style/Circle.js'

import { PRINT_DPI_COMPENSATION } from '@/config/print.config'
import variables from '@/scss/variables-admin.module.scss'

const { red, mocassin, mocassinToRed1, mocassinToRed2, malibu, black, white } = variables
Expand Down Expand Up @@ -158,10 +159,9 @@ export const highlightPointStyle = new Style({
// Change a width according to the change of DPI (from the old geoadmin)
// Originally introduced here https://github.com/geoadmin/mf-geoadmin3/pull/3280
export function adjustWidth(width, dpi) {
if (!width) {
return
if (!width || isNaN(width) || !dpi || isNaN(dpi) || dpi <= 0) {
return 0
}
// 90 is choosen to compensate the difference DPI between the WMTS or WMS and the print DPI used.
// The number is from the old geoadmin (see the link above)
return (width * 90) / dpi

return (width * PRINT_DPI_COMPENSATION) / dpi
}
95 changes: 46 additions & 49 deletions tests/cypress/tests-e2e/print.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,40 +426,46 @@ describe('Testing print', () => {
).to.lessThan(1.5) // thinner than the drawn in the OL map.
})
})
/** We need to ensure the structure of the query sent is correct */
it('should send a print request correctly to mapfishprint (icon and label)', () => {
startPrintWithKml('print/label.kml')

cy.wait('@printRequest').then((interception) => {
expect(interception.request.body).to.haveOwnProperty('layout')
expect(interception.request.body['layout']).to.equal('1. A4 landscape')
expect(interception.request.body).to.haveOwnProperty('format')
expect(interception.request.body['format']).to.equal('pdf')

const attributes = interception.request.body.attributes
expect(attributes).to.haveOwnProperty('printLegend')
expect(attributes['printLegend']).to.equals(0)
expect(attributes).to.haveOwnProperty('qrimage')
expect(attributes['qrimage']).to.contains(
encodeURIComponent('https://s.geo.admin.ch/0000000')
)

expect(attributes).to.haveOwnProperty('map')
const mapAttributes = attributes.map
expect(mapAttributes['scale']).to.equals(5000)
expect(mapAttributes['dpi']).to.equals(254)
expect(mapAttributes['projection']).to.equals('EPSG:2056')

expect(mapAttributes).to.haveOwnProperty('scale')
expect(mapAttributes).to.haveOwnProperty('dpi')
expect(mapAttributes).to.haveOwnProperty('projection')

expect(mapAttributes).to.haveOwnProperty('layers')

const layers = mapAttributes.layers

expect(layers).to.be.an('array')
expect(layers).to.have.length(2)
expect(layers[0]['type']).to.equals('geojson')
expect(layers[0]['geoJson']['features']).to.have.length(1)
expect(layers[0]['geoJson']['features'][0]['properties']).to.haveOwnProperty(

const geoJsonLayer = layers[0]

expect(geoJsonLayer).to.haveOwnProperty('type')
expect(geoJsonLayer).to.haveOwnProperty('geoJson')
expect(geoJsonLayer).to.haveOwnProperty('style')
expect(geoJsonLayer['geoJson']).to.haveOwnProperty('features')
expect(geoJsonLayer['geoJson']['features'][0]).to.haveOwnProperty('properties')
expect(geoJsonLayer['geoJson']['features'][0]['properties']).to.haveOwnProperty(
'_mfp_style'
)
expect(layers[0]['geoJson']['features'][0]['properties']['_mfp_style']).to.equal(
'1'
expect(geoJsonLayer['style']).to.haveOwnProperty("[_mfp_style = '1']")
expect(geoJsonLayer['style']["[_mfp_style = '1']"]).to.haveOwnProperty(
'symbolizers'
)
expect(layers[0]['style']).to.haveOwnProperty("[_mfp_style = '1']")

const symbolizers = layers[0]['style']["[_mfp_style = '1']"]['symbolizers']
const pointSymbol = symbolizers[0]
Expand All @@ -468,19 +474,13 @@ describe('Testing print', () => {
type: 'point',
externalGraphic: '001-marker@1x-255,0,0.png', // suffix only
graphicWidth: 19.133858267716537,
graphicXOffset: -8.503937007874017,
graphicYOffset: -8.503937007874017,
}

for (const attribute in pointSymbolAttributes) {
expect(pointSymbol).to.haveOwnProperty(attribute)
}
expect(
pointSymbol['externalGraphic'].endsWith(
pointSymbolAttributes['externalGraphic']
)
).to.be.true
expect(
pointSymbol['graphicWidth'] - pointSymbolAttributes['graphicWidth']
).to.lessThan(0.1)

const textSymbol = symbolizers[1]
const textSymbolAttributes = {
Expand All @@ -493,7 +493,6 @@ describe('Testing print', () => {
}
for (const attribute in textSymbolAttributes) {
expect(textSymbol).to.haveOwnProperty(attribute)
expect(textSymbol[attribute]).to.equal(textSymbolAttributes[attribute])
}
})
})
Expand All @@ -502,69 +501,67 @@ describe('Testing print', () => {

cy.wait('@printRequest').then((interception) => {
expect(interception.request.body).to.haveOwnProperty('layout')
expect(interception.request.body['layout']).to.equal('1. A4 landscape')
expect(interception.request.body).to.haveOwnProperty('format')
expect(interception.request.body['format']).to.equal('pdf')

const attributes = interception.request.body.attributes
expect(attributes).to.haveOwnProperty('printLegend')
expect(attributes['printLegend']).to.equals(0)
expect(attributes).to.haveOwnProperty('qrimage')
expect(attributes['qrimage']).to.contains(
encodeURIComponent('https://s.geo.admin.ch/0000000')
)

expect(attributes).to.haveOwnProperty('map')
const mapAttributes = attributes.map
expect(mapAttributes['scale']).to.equals(5000)
expect(mapAttributes['dpi']).to.equals(254)
expect(mapAttributes['projection']).to.equals('EPSG:2056')

expect(mapAttributes).to.haveOwnProperty('scale')
expect(mapAttributes).to.haveOwnProperty('dpi')
expect(mapAttributes).to.haveOwnProperty('projection')

expect(mapAttributes).to.haveOwnProperty('layers')

const layers = mapAttributes.layers

expect(layers).to.be.an('array')
expect(layers).to.have.length(2)
expect(layers[0]['type']).to.equals('geojson')
expect(layers[0]['geoJson']['features']).to.have.length(1)
expect(layers[0]['geoJson']['features'][0]['properties']).to.haveOwnProperty(

const geoJsonLayer = layers[0]

expect(geoJsonLayer).to.haveOwnProperty('type')
expect(geoJsonLayer).to.haveOwnProperty('geoJson')
expect(geoJsonLayer).to.haveOwnProperty('style')
expect(geoJsonLayer['geoJson']).to.haveOwnProperty('features')
expect(geoJsonLayer['geoJson']['features'][0]).to.haveOwnProperty('properties')
expect(geoJsonLayer['geoJson']['features'][0]['properties']).to.haveOwnProperty(
'_mfp_style'
)
expect(layers[0]['geoJson']['features'][0]['properties']['_mfp_style']).to.equal(
'1'
expect(geoJsonLayer['style']).to.haveOwnProperty("[_mfp_style = '1']")
expect(geoJsonLayer['style']["[_mfp_style = '1']"]).to.haveOwnProperty(
'symbolizers'
)
expect(layers[0]['style']).to.haveOwnProperty("[_mfp_style = '1']")

const symbolizers = layers[0]['style']["[_mfp_style = '1']"]['symbolizers']
const pointSymbol = symbolizers[0]
expect(pointSymbol).to.haveOwnProperty('type')
const pointSymbolAttributes = {
type: 'point',
externalGraphic: '001-marker@1x-255,0,0.png', // suffix only
graphicWidth: 17.007874015748033,
graphicWidth: 19.133858267716537,
graphicXOffset: -8.503937007874017,
graphicYOffset: -8.503937007874017,
}

for (const attribute in pointSymbolAttributes) {
expect(pointSymbol).to.haveOwnProperty(attribute)
if (attribute === 'externalGraphic') {
expect(pointSymbol[attribute].endsWith(pointSymbolAttributes[attribute])).to
.be.true
} else {
expect(pointSymbol[attribute]).to.equal(pointSymbolAttributes[attribute])
}
}

const textSymbol = symbolizers[1]
const textSymbolAttributes = {
type: 'text',
label: 'Old Label',
label: 'Sample Label',
fontFamily: 'Helvetica',
fontSize: '12px',
fontWeight: 'normal',
labelYOffset: 0,
labelYOffset: 44.75,
}
for (const attribute in textSymbolAttributes) {
expect(textSymbol).to.haveOwnProperty(attribute)
expect(textSymbol[attribute]).to.equal(textSymbolAttributes[attribute])
}
})
})
Expand Down

0 comments on commit 7bd41b0

Please sign in to comment.