Bug in PAL labelling over the dateline

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in PAL labelling over the dateline

Jeremy Palmer-2
Hi All,

I've discovered a label rendering bug for geographic vectors layer when the mapcanvas bound spans the dateline. To replicate this do the following:

1) Add a vector layer with a geographic coordinate system, preferably one that already spans the dateline. I have attached a simple Shapefile.
2) Enable PAL labelling on this layer.
3) Turn on OTF projections. Make sure you use a coordinate system that will work for data over the dateline. I used EPSG:2193 for the attached Shapefile.
4) Zoom to a projected extent that will ensure that layer will span +/- 180 dateline.

This should raise a generic exception that is caught by QGIS, which leaves the application in state that is not usable. This exception is actually a re-throw of PalException::LayerExists.

At this stage I have debugged the application and determined this:

* When otf transformed are enabled, layer is has a geographic coordinate system and the canvas extents span the dateline two separate render operation will be initiated by QgsMapRenderer. See lines 475-419 in qgsmaprenderer.cpp

* Within the QgsMapRenderer::Render method the labelling engine is initialised, calls are made to the layer to draw the data, the labels are rendered and finally the labelling engine is cleaned up.

* Within the layer draw method the labelling engine prepares the layer for labelling i.e rendererContext.labelingEngine()->prepareLayer(...). In the context of the PAL label engine this sets the layer up for label rendering and adds metadata about the layer to the PAL object. i.e. Pal::addLayer(...)

* You can not add a duplicate layer to the PAL object otherwise it will throw a PalException::LayerExists exception. See line 186 of pal.cpp.

>From a simplistic point of view it seems to me that preparation of the label engine should occur within the QgsMapRenderer::Render method and not deeper down in the layer draw methods. I see that in the layer draw methods call the labelingEngine::prepareLayer() to determine if labelling is required and which fields are required for the data select. It seems that this information could be associated with the QgsRenderContext and set within QgsMapRenderer::Render() then and accessed within the layer draw.

Can someone please comment on this?

Thanks heaps,
Jeremy

______________________________________________________________________________________________________

This message contains information, which is confidential and may be subject to legal privilege.
If you are not the intended recipient, you must not peruse, use, disseminate, distribute or copy this message.
If you have received this message in error, please notify us immediately (Phone 0800 665 463 or [hidden email]) and destroy the original message.
LINZ accepts no responsibility for changes to this email, or for any attachments, after its transmission from LINZ.

Thank you.
______________________________________________________________________________________________________

_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer

dateline_polygon.zip (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in PAL labelling over the dateline

Marco Hugentobler-4
Hi Jeremy

Wouldn't it be easiest to, in pal.cpp, just ignore layers that are already
there instead of throwing an exception? And the same could be done for
features crossing the dateline in pal/layer.cpp:239

If the label initialisation would be done in QgsMapRenderer, it would be
necessary to change the layer interface to pass additional attribute indices
to fetch (e.g. necessary in case of data defined label settings).

Regards,
Marco


>Am Mittwoch, 6. Oktober 2010, um 08.03:58 schrieb Jeremy Palmer:
>Hi All,
>
>I've discovered a label rendering bug for geographic vectors layer when the
>mapcanvas bound spans the dateline. To replicate this do the following:
>
>1) Add a vector layer with a geographic coordinate system, preferably one
that already spans the dateline. I have attached a simple Shapefile.

>2) Enable PAL labelling on this layer.
>3) Turn on OTF projections. Make sure you use a coordinate system that will
>work for data over the dateline. I used EPSG:2193 for the attached Shapefile.
>4) Zoom to a projected extent that will ensure that layer will span +/- 180
>dateline.
>
>This should raise a generic exception that is caught by QGIS, which leaves
>the application in state that is not usable. This exception is actually a re-
>throw of PalException::LayerExists.
>
>At this stage I have debugged the application and determined this:
>
>* When otf transformed are enabled, layer is has a geographic coordinate
>system and the canvas extents span the dateline two separate render operation
>will be initiated by QgsMapRenderer. See lines 475-419 in qgsmaprenderer.cpp
>
>* Within the QgsMapRenderer::Render method the labelling engine is
>initialised, calls are made to the layer to draw the data, the labels are
>rendered and finally the labelling engine is cleaned up.
>
>* Within the layer draw method the labelling engine prepares the layer for
>labelling i.e rendererContext.labelingEngine()->prepareLayer(...). In the
>context of the PAL label engine this sets the layer up for label rendering
>and adds metadata about the layer to the PAL object. i.e. Pal::addLayer(...)
>
>* You can not add a duplicate layer to the PAL object otherwise it will throw
>a PalException::LayerExists exception. See line 186 of pal.cpp.
>
>From a simplistic point of view it seems to me that preparation of the label
>engine should occur within the QgsMapRenderer::Render method and not deeper
>down in the layer draw methods. I see that in the layer draw methods call the
>labelingEngine::prepareLayer() to determine if labelling is required and
>which fields are required for the data select. It seems that this information
>could be associated with the QgsRenderContext and set within
>QgsMapRenderer::Render() then and accessed within the layer draw.
>
>Can someone please comment on this?
>
>Thanks heaps,
>Jeremy



--
Dr. Marco Hugentobler
Sourcepole -  Linux & Open Source Solutions
Webereistrasse 66, 8134 Adliswil, Switzerland
[hidden email] http://www.sourcepole.ch
Technical Advisor QGIS Project Steering Committee
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

RE: Bug in PAL labelling over the dateline

Jeremy Palmer-2
Hi Marco,

Yes you are right, it's definitely easier and it doesn't mean changing the interfaces or too much code. So if you think this is the path of least resistant I'd be keen to fly with it.

Quick question: Is it ok to change the PAL library like this, or is it better to put an exception handler around the PAL calls. I'm just thinking about future merges of the PAL library into the QGIS source.

Cheers,
Jeremy

-----Original Message-----
From: Marco Hugentobler [mailto:[hidden email]]
Sent: Thursday, October 07, 2010 12:51 AM
To: [hidden email]
Subject: Re: [Qgis-developer] Bug in PAL labelling over the dateline

Hi Jeremy

Wouldn't it be easiest to, in pal.cpp, just ignore layers that are already
there instead of throwing an exception? And the same could be done for
features crossing the dateline in pal/layer.cpp:239

If the label initialisation would be done in QgsMapRenderer, it would be
necessary to change the layer interface to pass additional attribute indices
to fetch (e.g. necessary in case of data defined label settings).

Regards,
Marco


>Am Mittwoch, 6. Oktober 2010, um 08.03:58 schrieb Jeremy Palmer:
>Hi All,
>
>I've discovered a label rendering bug for geographic vectors layer when the
>mapcanvas bound spans the dateline. To replicate this do the following:
>
>1) Add a vector layer with a geographic coordinate system, preferably one
that already spans the dateline. I have attached a simple Shapefile.

>2) Enable PAL labelling on this layer.
>3) Turn on OTF projections. Make sure you use a coordinate system that will
>work for data over the dateline. I used EPSG:2193 for the attached Shapefile.
>4) Zoom to a projected extent that will ensure that layer will span +/- 180
>dateline.
>
>This should raise a generic exception that is caught by QGIS, which leaves
>the application in state that is not usable. This exception is actually a re-
>throw of PalException::LayerExists.
>
>At this stage I have debugged the application and determined this:
>
>* When otf transformed are enabled, layer is has a geographic coordinate
>system and the canvas extents span the dateline two separate render operation
>will be initiated by QgsMapRenderer. See lines 475-419 in qgsmaprenderer.cpp
>
>* Within the QgsMapRenderer::Render method the labelling engine is
>initialised, calls are made to the layer to draw the data, the labels are
>rendered and finally the labelling engine is cleaned up.
>
>* Within the layer draw method the labelling engine prepares the layer for
>labelling i.e rendererContext.labelingEngine()->prepareLayer(...). In the
>context of the PAL label engine this sets the layer up for label rendering
>and adds metadata about the layer to the PAL object. i.e. Pal::addLayer(...)
>
>* You can not add a duplicate layer to the PAL object otherwise it will throw
>a PalException::LayerExists exception. See line 186 of pal.cpp.
>
>From a simplistic point of view it seems to me that preparation of the label
>engine should occur within the QgsMapRenderer::Render method and not deeper
>down in the layer draw methods. I see that in the layer draw methods call the
>labelingEngine::prepareLayer() to determine if labelling is required and
>which fields are required for the data select. It seems that this information
>could be associated with the QgsRenderContext and set within
>QgsMapRenderer::Render() then and accessed within the layer draw.
>
>Can someone please comment on this?
>
>Thanks heaps,
>Jeremy



--
Dr. Marco Hugentobler
Sourcepole -  Linux & Open Source Solutions
Webereistrasse 66, 8134 Adliswil, Switzerland
[hidden email] http://www.sourcepole.ch
Technical Advisor QGIS Project Steering Committee

______________________________________________________________________________________________________

This message contains information, which is confidential and may be subject to legal privilege.
If you are not the intended recipient, you must not peruse, use, disseminate, distribute or copy this message.
If you have received this message in error, please notify us immediately (Phone 0800 665 463 or [hidden email]) and destroy the original message.
LINZ accepts no responsibility for changes to this email, or for any attachments, after its transmission from LINZ.

Thank you.
______________________________________________________________________________________________________
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Bug in PAL labelling over the dateline

Marco Hugentobler-4
Hi Jeremy

Applied in r14352, thank you for the good problem description and the
testdata.

> Quick question: Is it ok to change the PAL library like this, or is it
> better to put an exception handler around the PAL calls. I'm just thinking
> about future merges of the PAL library into the QGIS source.

There are already other PAL changes in the QGIS source tree that need to be
merged back into PAL, e.g. Martins curved labeling or the possibility to force
certain label positions for data defined labeling. So agreed, we should do that
soon to stay in sync with PAL.

Regards,
Marco

Am Mittwoch, 6. Oktober 2010, um 22.45:29 schrieb Jeremy Palmer:

> Hi Marco,
>
> Yes you are right, it's definitely easier and it doesn't mean changing the
> interfaces or too much code. So if you think this is the path of least
> resistant I'd be keen to fly with it.
>
> Quick question: Is it ok to change the PAL library like this, or is it
> better to put an exception handler around the PAL calls. I'm just thinking
> about future merges of the PAL library into the QGIS source.
>
> Cheers,
> Jeremy
>
> -----Original Message-----
> From: Marco Hugentobler [mailto:[hidden email]]
> Sent: Thursday, October 07, 2010 12:51 AM
> To: [hidden email]
> Subject: Re: [Qgis-developer] Bug in PAL labelling over the dateline
>
> Hi Jeremy
>
> Wouldn't it be easiest to, in pal.cpp, just ignore layers that are already
> there instead of throwing an exception? And the same could be done for
> features crossing the dateline in pal/layer.cpp:239
>
> If the label initialisation would be done in QgsMapRenderer, it would be
> necessary to change the layer interface to pass additional attribute
> indices to fetch (e.g. necessary in case of data defined label settings).
>
> Regards,
> Marco
>
> >Am Mittwoch, 6. Oktober 2010, um 08.03:58 schrieb Jeremy Palmer:
> >Hi All,
> >
> >I've discovered a label rendering bug for geographic vectors layer when
> >the mapcanvas bound spans the dateline. To replicate this do the
> >following:
> >
> >1) Add a vector layer with a geographic coordinate system, preferably one
>
> that already spans the dateline. I have attached a simple Shapefile.
>
> >2) Enable PAL labelling on this layer.
> >3) Turn on OTF projections. Make sure you use a coordinate system that
> >will work for data over the dateline. I used EPSG:2193 for the attached
> >Shapefile. 4) Zoom to a projected extent that will ensure that layer will
> >span +/- 180 dateline.
> >
> >This should raise a generic exception that is caught by QGIS, which leaves
> >the application in state that is not usable. This exception is actually a
> >re- throw of PalException::LayerExists.
> >
> >At this stage I have debugged the application and determined this:
> >
> >* When otf transformed are enabled, layer is has a geographic coordinate
> >system and the canvas extents span the dateline two separate render
> >operation will be initiated by QgsMapRenderer. See lines 475-419 in
> >qgsmaprenderer.cpp
> >
> >* Within the QgsMapRenderer::Render method the labelling engine is
> >initialised, calls are made to the layer to draw the data, the labels are
> >rendered and finally the labelling engine is cleaned up.
> >
> >* Within the layer draw method the labelling engine prepares the layer for
> >labelling i.e rendererContext.labelingEngine()->prepareLayer(...). In the
> >context of the PAL label engine this sets the layer up for label rendering
> >and adds metadata about the layer to the PAL object. i.e.
> >Pal::addLayer(...)
> >
> >* You can not add a duplicate layer to the PAL object otherwise it will
> >throw a PalException::LayerExists exception. See line 186 of pal.cpp.
> >
> >From a simplistic point of view it seems to me that preparation of the
> >label engine should occur within the QgsMapRenderer::Render method and
> >not deeper down in the layer draw methods. I see that in the layer draw
> >methods call the labelingEngine::prepareLayer() to determine if labelling
> >is required and which fields are required for the data select. It seems
> >that this information could be associated with the QgsRenderContext and
> >set within
> >QgsMapRenderer::Render() then and accessed within the layer draw.
> >
> >Can someone please comment on this?
> >
> >Thanks heaps,
> >Jeremy


--
Dr. Marco Hugentobler
Sourcepole -  Linux & Open Source Solutions
Webereistrasse 66, 8134 Adliswil, Switzerland
[hidden email] http://www.sourcepole.ch
Technical Advisor QGIS Project Steering Committee
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Bug in PAL labelling over the dateline

luca_manganelli
[hidden email] scritti il 07/10/2010 08.40.07

> There are already other PAL changes in the QGIS source tree that need to
be
> merged back into PAL, e.g. Martins curved labeling or the possibility to
force
> certain label positions for data defined labeling. So agreed, we
> should do that

I don't understand one thing: is the PAL library integrated in QGIS main
trunk or on a specific branch?

_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Bug in PAL labelling over the dateline

Martin Dobias
In reply to this post by Marco Hugentobler-4
On Thu, Oct 7, 2010 at 8:40 AM, Marco Hugentobler
<[hidden email]> wrote:

> Hi Jeremy
>
> Applied in r14352, thank you for the good problem description and the
> testdata.
>
>> Quick question: Is it ok to change the PAL library like this, or is it
>> better to put an exception handler around the PAL calls. I'm just thinking
>> about future merges of the PAL library into the QGIS source.
>
> There are already other PAL changes in the QGIS source tree that need to be
> merged back into PAL, e.g. Martins curved labeling or the possibility to force
> certain label positions for data defined labeling. So agreed, we should do that
> soon to stay in sync with PAL.

AFAIK there has been no development within PAL for about two years
except for some small fixes - see [1]. So effectively development of
PAL happens only within the copy of PAL in QGIS repository. Therefore
it should be quite easy to merge all the changes back, the only
question is whether the original authors will be willing to merge all
our changes.

[1] http://geosysin.iict.ch/pal-trac/log/trunk

Martin
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

Re: Bug in PAL labelling over the dateline

Martin Dobias
In reply to this post by luca_manganelli
On Thu, Oct 7, 2010 at 9:00 AM,  <[hidden email]> wrote:

> [hidden email] scritti il 07/10/2010 08.40.07
>
>> There are already other PAL changes in the QGIS source tree that need to
> be
>> merged back into PAL, e.g. Martins curved labeling or the possibility to
> force
>> certain label positions for data defined labeling. So agreed, we
>> should do that
>
> I don't understand one thing: is the PAL library integrated in QGIS main
> trunk or on a specific branch?

Yes, it's in trunk: src/core/pal directory

Martin
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer
Reply | Threaded
Open this post in threaded view
|

RE: Bug in PAL labelling over the dateline

Jeremy Palmer-2
In reply to this post by Marco Hugentobler-4
Hi Marco,

Thanks for the fix!

Cheers,
Jeremy

-----Original Message-----
From: Marco Hugentobler [mailto:[hidden email]]
Sent: Thursday, October 07, 2010 7:40 PM
To: [hidden email]
Subject: Re: [Qgis-developer] Bug in PAL labelling over the dateline

Hi Jeremy

Applied in r14352, thank you for the good problem description and the
testdata.

> Quick question: Is it ok to change the PAL library like this, or is it
> better to put an exception handler around the PAL calls. I'm just thinking
> about future merges of the PAL library into the QGIS source.

There are already other PAL changes in the QGIS source tree that need to be
merged back into PAL, e.g. Martins curved labeling or the possibility to force
certain label positions for data defined labeling. So agreed, we should do that
soon to stay in sync with PAL.

Regards,
Marco

Am Mittwoch, 6. Oktober 2010, um 22.45:29 schrieb Jeremy Palmer:

> Hi Marco,
>
> Yes you are right, it's definitely easier and it doesn't mean changing the
> interfaces or too much code. So if you think this is the path of least
> resistant I'd be keen to fly with it.
>
> Quick question: Is it ok to change the PAL library like this, or is it
> better to put an exception handler around the PAL calls. I'm just thinking
> about future merges of the PAL library into the QGIS source.
>
> Cheers,
> Jeremy
>
> -----Original Message-----
> From: Marco Hugentobler [mailto:[hidden email]]
> Sent: Thursday, October 07, 2010 12:51 AM
> To: [hidden email]
> Subject: Re: [Qgis-developer] Bug in PAL labelling over the dateline
>
> Hi Jeremy
>
> Wouldn't it be easiest to, in pal.cpp, just ignore layers that are already
> there instead of throwing an exception? And the same could be done for
> features crossing the dateline in pal/layer.cpp:239
>
> If the label initialisation would be done in QgsMapRenderer, it would be
> necessary to change the layer interface to pass additional attribute
> indices to fetch (e.g. necessary in case of data defined label settings).
>
> Regards,
> Marco
>
> >Am Mittwoch, 6. Oktober 2010, um 08.03:58 schrieb Jeremy Palmer:
> >Hi All,
> >
> >I've discovered a label rendering bug for geographic vectors layer when
> >the mapcanvas bound spans the dateline. To replicate this do the
> >following:
> >
> >1) Add a vector layer with a geographic coordinate system, preferably one
>
> that already spans the dateline. I have attached a simple Shapefile.
>
> >2) Enable PAL labelling on this layer.
> >3) Turn on OTF projections. Make sure you use a coordinate system that
> >will work for data over the dateline. I used EPSG:2193 for the attached
> >Shapefile. 4) Zoom to a projected extent that will ensure that layer will
> >span +/- 180 dateline.
> >
> >This should raise a generic exception that is caught by QGIS, which leaves
> >the application in state that is not usable. This exception is actually a
> >re- throw of PalException::LayerExists.
> >
> >At this stage I have debugged the application and determined this:
> >
> >* When otf transformed are enabled, layer is has a geographic coordinate
> >system and the canvas extents span the dateline two separate render
> >operation will be initiated by QgsMapRenderer. See lines 475-419 in
> >qgsmaprenderer.cpp
> >
> >* Within the QgsMapRenderer::Render method the labelling engine is
> >initialised, calls are made to the layer to draw the data, the labels are
> >rendered and finally the labelling engine is cleaned up.
> >
> >* Within the layer draw method the labelling engine prepares the layer for
> >labelling i.e rendererContext.labelingEngine()->prepareLayer(...). In the
> >context of the PAL label engine this sets the layer up for label rendering
> >and adds metadata about the layer to the PAL object. i.e.
> >Pal::addLayer(...)
> >
> >* You can not add a duplicate layer to the PAL object otherwise it will
> >throw a PalException::LayerExists exception. See line 186 of pal.cpp.
> >
> >From a simplistic point of view it seems to me that preparation of the
> >label engine should occur within the QgsMapRenderer::Render method and
> >not deeper down in the layer draw methods. I see that in the layer draw
> >methods call the labelingEngine::prepareLayer() to determine if labelling
> >is required and which fields are required for the data select. It seems
> >that this information could be associated with the QgsRenderContext and
> >set within
> >QgsMapRenderer::Render() then and accessed within the layer draw.
> >
> >Can someone please comment on this?
> >
> >Thanks heaps,
> >Jeremy


--
Dr. Marco Hugentobler
Sourcepole -  Linux & Open Source Solutions
Webereistrasse 66, 8134 Adliswil, Switzerland
[hidden email] http://www.sourcepole.ch
Technical Advisor QGIS Project Steering Committee

______________________________________________________________________________________________________

This message contains information, which is confidential and may be subject to legal privilege.
If you are not the intended recipient, you must not peruse, use, disseminate, distribute or copy this message.
If you have received this message in error, please notify us immediately (Phone 0800 665 463 or [hidden email]) and destroy the original message.
LINZ accepts no responsibility for changes to this email, or for any attachments, after its transmission from LINZ.

Thank you.
______________________________________________________________________________________________________
_______________________________________________
Qgis-developer mailing list
[hidden email]
http://lists.osgeo.org/mailman/listinfo/qgis-developer