Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in Dynamic Temporal Controller, "Include Start, Include End" no longer works #48946

Closed
2 tasks done
CoryAlbrecht opened this issue Jun 9, 2022 · 15 comments · Fixed by #49865
Closed
2 tasks done
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Temporal Temporal filtering or animation

Comments

@CoryAlbrecht
Copy link

What is the bug or the crash?

The "Include End" setting in temporal controls for a layer no longer works.

image

image

image

image

Steps to reproduce the issue

  1. Load layers with temporal data.
  2. Turn on the DTC for the layer and set to "Include Start, Include End"
  3. Using the temporal navigator go back and forth around the end time of a feature
  4. See how the feature disappears for the last time unit

Versions

<style type="text/css"> p, li { white-space: pre-wrap; } </style>
QGIS version 3.25.0-Master QGIS code revision a2001fd
Qt version 5.12.8
Python version 3.8.10
GDAL/OGR version 3.0.4
PROJ version 6.3.1
EPSG Registry database version v9.8.6 (2020-01-22)
Compiled against GEOS 3.8.0-CAPI-1.13.1 Running against GEOS 3.8.0-CAPI-1.13.1
SQLite version 3.31.1
PDAL version 2.0.1
PostgreSQL client version 12.11 (Ubuntu 12.11-0ubuntu0.20.04.1)
SpatiaLite version 4.3.0a
QWT version 6.1.4
QScintilla2 version 2.11.2
OS version Ubuntu 20.04.4 LTS
       
This copy of QGIS writes debugging output.
       
Active Python plugins
QGIS3-getWKT 1.4
geometry_paster 0.1.1
postgisQueryBuilder 2.0.1
actions_for_relations 1.2.0
ClipMultipleLayers 3.2.0
SelectByRelationship 0.3.3
cartography_tools 1.2.1
bettereditor 1.5.0
quick_map_services 0.19.29
GTFS-GO-master 3.0.1
db-style-manager 0.8
splitmultipart 1.0.0
ordered_relation_editor 0.2.7
pg_raster_import 1.0.10
grassprovider 2.12.99
processing 2.12.99
MetaSearch 0.3.6
db_manager 0.1.20
sagaprovider 2.12.99
QGIS version 3.25.0-Master QGIS code revision [a2001fd](https://github.com/qgis/QGIS/commit/a2001fdd69) Qt version 5.12.8 Python version 3.8.10 GDAL/OGR version 3.0.4 PROJ version 6.3.1 EPSG Registry database version v9.8.6 (2020-01-22) Compiled against GEOS 3.8.0-CAPI-1.13.1 Running against GEOS 3.8.0-CAPI-1.13.1 SQLite version 3.31.1 PDAL version 2.0.1 PostgreSQL client version 12.11 (Ubuntu 12.11-0ubuntu0.20.04.1) SpatiaLite version 4.3.0a QWT version 6.1.4 QScintilla2 version 2.11.2 OS version Ubuntu 20.04.4 LTS

This copy of QGIS writes debugging output.

Active Python plugins
QGIS3-getWKT
1.4
geometry_paster
0.1.1
postgisQueryBuilder
2.0.1
actions_for_relations
1.2.0
ClipMultipleLayers
3.2.0
SelectByRelationship
0.3.3
cartography_tools
1.2.1
bettereditor
1.5.0
quick_map_services
0.19.29
GTFS-GO-master
3.0.1
db-style-manager
0.8
splitmultipart
1.0.0
ordered_relation_editor
0.2.7
pg_raster_import
1.0.10
grassprovider
2.12.99
processing
2.12.99
MetaSearch
0.3.6
db_manager
0.1.20
sagaprovider
2.12.99

Supported QGIS version

  • I'm running a supported QGIS version according to the roadmap.

New profile

  • I tried with a new QGIS profile

Additional context

Ubuntu 20.04 LTS

@CoryAlbrecht CoryAlbrecht added the Bug Either a bug report, or a bug fix. Let's hope for the latter! label Jun 9, 2022
@CoryAlbrecht
Copy link
Author

I built 3.22.5 and it does not have this problem.

@nyalldawson
Copy link
Collaborator

This code hasn't been touched recently, and is well protected by regression tests. The issue must lie deeper than the temporal controller... Are you able to provide a sample project for testing?

@nyalldawson nyalldawson added the Feedback Waiting on the submitter for answers label Jun 9, 2022
@Pedro-Murteira Pedro-Murteira added the Temporal Temporal filtering or animation label Jun 13, 2022
@github-actions
Copy link

The QGIS project highly values your report and would love to see it addressed. However, this issue has been left in feedback mode for the last 14 days and is being automatically marked as "stale".
If you would like to continue with this issue, please provide any missing information or answer any open questions. If you could resolve the issue yourself meanwhile, please leave a note for future readers with the same problem and close the issue.
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this issue.
If there is no further activity on this issue, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jun 28, 2022
@CoryAlbrecht
Copy link
Author

Small project plus profile made with latest "nightly" build downloaded today. Profile name is test-1.

qgis-test.tar.gz

If you unzip this on a Linux distrubition while in ${HOME} it will add ~/Downloads/qgis-test/ with the project and ~/.local/share/QGIS/QGIS3/profiles/test-1/ with the profile. Steps proceed with that assumption.

  1. Start with /usr/bin/qgis --profile test-1
  2. Load the project ~/Downloads/qgis-test/test-1.qgz
  3. Click the Rewind to start button in the Temporal Controller to get to 2021-12-31
  4. Click the Go to next frame repeatedly taking note of the shapes as they appear and disappear
  5. A red shape appears on 2022-01-01 displaying the range of dates during which it should be seen, 2022-01-01 to 2022-01-07
  6. A green shape appears on 2022-01-02 displaying the range of dates during which it should be seen, 2022-01-02 to 2022-01-08
  7. A blue shape appears on 2022-01-03 displaying the range of dates during which it should be seen, 2022-01-03 to 2022-01-09
  8. Keep clicking until 2022-01-06, all three shapes stay visible
  9. Click once more to 2022-01-07, notice that the red shape disappears but it should shay visible because Dynamic Temporal Control is defined as Include Start, Include End
  10. Click once more to 2022-01-08, notice that the green shape disappears but it should shay visible because Dynamic Temporal Control is defined as Include Start, Include End
  11. Click once more to 2022-01-09, notice that the blue shape disappears but it should shay visible because Dynamic Temporal Control is defined as Include Start, Include End

Video of using the project and profile:

qgis-test-1-2022-07-03_17.36.23.mp4
<style type="text/css"> p, li { white-space: pre-wrap; } </style>
QGIS version 3.27.0-Master QGIS code revision 0e23467
Qt version 5.12.8
Python version 3.8.10
GDAL/OGR version 3.0.4
PROJ version 6.3.1
EPSG Registry database version v9.8.6 (2020-01-22)
Compiled against GEOS 3.8.0-CAPI-1.13.1 Running against GEOS 3.8.0-CAPI-1.13.1
SQLite version 3.31.1
PDAL version 2.0.1
PostgreSQL client version 12.11 (Ubuntu 12.11-0ubuntu0.20.04.1)
SpatiaLite version 4.3.0a
QWT version 6.1.4
QScintilla2 version 2.11.2
OS version Ubuntu 20.04.4 LTS
       
This copy of QGIS writes debugging output.
       
Active Python plugins
grassprovider 2.12.99
processing 2.12.99
MetaSearch 0.3.6
db_manager 0.1.20
sagaprovider 2.12.99
QGIS version 3.27.0-Master QGIS code revision [0e23467](https://github.com/qgis/QGIS/commit/0e23467727) Qt version 5.12.8 Python version 3.8.10 GDAL/OGR version 3.0.4 PROJ version 6.3.1 EPSG Registry database version v9.8.6 (2020-01-22) Compiled against GEOS 3.8.0-CAPI-1.13.1 Running against GEOS 3.8.0-CAPI-1.13.1 SQLite version 3.31.1 PDAL version 2.0.1 PostgreSQL client version 12.11 (Ubuntu 12.11-0ubuntu0.20.04.1) SpatiaLite version 4.3.0a QWT version 6.1.4 QScintilla2 version 2.11.2 OS version Ubuntu 20.04.4 LTS

This copy of QGIS writes debugging output.

Active Python plugins
grassprovider
2.12.99
processing
2.12.99
MetaSearch
0.3.6
db_manager
0.1.20
sagaprovider
2.12.99

@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jul 3, 2022
@CoryAlbrecht
Copy link
Author

CoryAlbrecht commented Jul 3, 2022

@nyalldawson Here is the request example project where it happens with the latest nightly build from the repo.
qgis-test.tar.gz

@Pedro-Murteira Pedro-Murteira removed the Feedback Waiting on the submitter for answers label Jul 4, 2022
@CoryAlbrecht
Copy link
Author

Has anybody had a chance to work on this?

@CoryAlbrecht
Copy link
Author

Sorry, I thought I had attached the file, but it's there now.

@CoryAlbrecht
Copy link
Author

Bump

1 similar comment
@CoryAlbrecht
Copy link
Author

Bump

@rduivenvoorde
Copy link
Contributor

rduivenvoorde commented Aug 20, 2022

Did some research,

Indeed 3.22 and master behave differently (master is wrong)

by looking at the filters created I see in our debug info:


master:
WITH end (including)
(to_datetime( "start" ) < make_datetime(2022,1,2,0,0,0) OR to_datetime( "start" ) IS NULL) AND ("end" >= make_datetime(2022,1,1,0,0,0) OR "end" IS NULL)
without
(to_datetime( "start" ) < make_datetime(2022,1,2,0,0,0) OR to_datetime( "start" ) IS NULL) AND ("end" > make_datetime(2022,1,1,0,0,0) OR "end" IS NULL)


3.22:
WITH end (including)
("start" < make_datetime(2022,1,2,0,0,0) OR "start" IS NULL) AND ("end" >= make_datetime(2022,1,1,0,0,0) OR "end" IS NULL)
and WITHOUT end
("start" < make_datetime(2022,1,2,0,0,0) OR "start" IS NULL) AND ("end" > make_datetime(2022,1,1,0,0,0) OR "end" IS NULL)


Some remarks:

  • note that the smaller/larger signs are still OK
  • but a to_datetime( ) was added (to do a cast of START) in master
  • BUT apparently only for the start, not at the end ... (have to be fixed to, I think)

It looks (to me) that this commit
e12bf44
added that,
in this pull
#48497
from @elpaso to add WMS temporal support for QGIS server

Apparently our tests did not find this.
It can also be specific to shp files (as these don't actually have datetime type fields)

Have to dive into the actual received value, or maybe somebody can tell if the this casting is actually an issue

https://github.com/qgis/QGIS/blob/master/src/core/vector/qgsvectorlayertemporalproperties.cpp#L450

@rduivenvoorde
Copy link
Contributor

Not sure, what exactly is the issue, but there is a (logical) difference if you take a 'date' from a shape (as said: a shp does not have a datetime, only dates) or if you cast it to a datetime:

# Taking start as example but could be end to:
# below all as seen as in expression editor

"start" (value in shp)                 <date: 2022-01-01>
to_datetime("start")                    <datetime: 2022-01-01 00:00:00 (CET)>
make_datetime(2022,1,1,0,0,0)  <datetime: 2022-01-01 00:00:00 (CET)>

"start" < make_datetime(2022, 1,1,0,0,0)
1 / True
"start" <= make_datetime(2022, 1,1,0,0,0)
1 / True
"start" > make_datetime(2022, 1,1,0,0,0)
0 / False
"start" >= make_datetime(2022, 1,1,0,0,0)
0 / False

to_datetime("start") < make_datetime(2022, 1,1,0,0,0)  
0 / False
to_datetime("start") <= make_datetime(2022,1,1,0,0,0)
1 / True
to_datetime("start") > make_datetime(2022, 1,1,0,0,0)
0 / False
to_datetime("start") >= make_datetime(2022, 1,1,0,0,0)
1 / True

to_date("start") < make_datetime(2022, 1,1,0,0,0)
1 / True
to_date("start") <= make_datetime(2022, 1,1,0,0,0)
1 / True
to_date("start") > make_datetime(2022, 1,1,0,0,0)
0 / False
to_date("start") >= make_datetime(2022, 1,1,0,0,0)
0 / False

@rduivenvoorde
Copy link
Contributor

rduivenvoorde commented Aug 20, 2022

Ok, master and 3.22 are behaving both OK/same if I change

QgsExpression::quotedColumnRef( mEndFieldName )

to

dateTimefieldCast( mEndFieldName ),

on line https://github.com/qgis/QGIS/blob/master/src/core/vector/qgsvectorlayertemporalproperties.cpp#L559
I think it was just missed when adding the (valuable!) casting.
Will do a PR and try if I can create/add a test so this get tested.

@CoryAlbrecht
Copy link
Author

Thank you for finding that @rduivenvoorde

@CoryAlbrecht
Copy link
Author

Hey @rduivenvoorde, I just noticed something new while using 3.22.4 from the Ubuntu official repos also has a very similar bug in the attribute table window. When I advance the DTC forward to the very last day of a feature, the feature disappears from the list in the attribute table. Same behaviour, from what I see, as the canvas version of the bug.

@rduivenvoorde
Copy link
Contributor

That is expected, as it uses the same 'filter'-string.... I think.
I'm waiting for the fix to be merged, or somebody proposes some usable tests to be added by me first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Temporal Temporal filtering or animation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants