OpenLayers OpenLayers

Ticket #1675 (closed feature: fixed)

Opened 2 years ago

Last modified 2 years ago

New vector rendering for better performance and less renderer specific limitations

Reported by: ahocevar Assigned to:
Priority: critical Milestone: 2.7 Release
Component: Renderer Version: 2.6
Keywords: Cc:
State: Complete

Description

This is a meta ticket for a partial rewrite of the vector rendering subsystem which fixes #1656, #1631, #1431.

The attached patch makes the following changes:

  • Only draw features in visible extent also for SVG (already introduced for VML in #1602). The performance test in tests/manual/vector-features-performance.html shows that performance will be better in most cases.
  • Layer.Vector gets notified by the renderer of unrendered (or not rendered as intended) features. This includes all features that do not intersect the map extent, plus all features that had to be modified to meet the SVG renderer's inValidRange requirements. The layer stores these features in the unrenderedFeatures hash.
  • When panning the map, the features from the unrenderedFeatures hash will be drawn. This was done to fix #1431 and #1656. It also ensures that only features that are not on the map yet need to be redrawn after panning, which improves performance significantly compared to the patches attached to #1656.
  • Renderer.SVG modifies LineString and LinearRing geometries that have some vertices outside the valid range. These features get clipped at the inValidRange bounds (which is always outside the visible extent), so they can be rendered correctly. A new acceptance test in tests/manual/clip-features-svg.html shows the results. This fixes issues with features with a huge extent, of which only a part is visible in the map viewport.
  • Renderer.SVG now uses a "translate" coordinate transformation instead of setting a new viewport for the renderer root when panning the map. This will give features that would be outside the valid range otherwise a chance to move inside the valid range when they get visible in the map viewport. According to the (sketchy) documentation of the Cairo issue referenced in #669, the valid coordinates range applies to the coordinates after transformation, which makes that possible.

Attachments

1675-r7728-A0.patch (54.7 kB) - added by ahocevar on 08/12/08 07:16:44.
1675-r7728-A1.patch (54.7 kB) - added by ahocevar on 08/16/08 14:25:58.
fixed a small but fatal typo that would have prevented features from being removed properly
1675-r7782-A1.patch (54.2 kB) - added by ahocevar on 08/18/08 08:32:11.
updated patch (no functional changes, just reflects trunk modifications)
1675-r7919-A1.patch (55.4 kB) - added by ahocevar on 09/01/08 17:51:56.
no functional changes, just another new patch because the old one would not apply any more due to trunk modifications

Change History

08/12/08 06:26:46 changed by ahocevar

The above patch contains unit and acceptance tests. All tests (except for those already failing on trunk) pass in FF2, FF3, IE6, IE7, Safari3 and Opera9.

08/12/08 07:16:44 changed by ahocevar

  • attachment 1675-r7728-A0.patch added.

08/16/08 14:25:58 changed by ahocevar

  • attachment 1675-r7728-A1.patch added.

fixed a small but fatal typo that would have prevented features from being removed properly

08/18/08 08:32:11 changed by ahocevar

  • attachment 1675-r7782-A1.patch added.

updated patch (no functional changes, just reflects trunk modifications)

09/01/08 16:49:12 changed by ahocevar

This patch will also close #1709.

09/01/08 17:51:56 changed by ahocevar

  • attachment 1675-r7919-A1.patch added.

no functional changes, just another new patch because the old one would not apply any more due to trunk modifications

(follow-up: ↓ 6 ) 09/02/08 02:55:53 changed by elemoine

  • state changed from Review to Commit.

The patch looks very good to me.

My review comments:

lib/OpenLayers/Renderer/Elements.js

  • line 482: OpenLayers.Util.getElement should be used as opposed to document.getElementById
  • line 504 and 568: geomety -> geometry
  • line 568: the comment is incomplete, "null if parts of the geometry couldn't be drawn" is missing
  • line 743: isn't "or null if the renderer could not draw all components of the rectangle" missing here too? Maybe not...

lib/OpenLayers/Layer/Vector.js

  • line 112: feature.is -> feature.id

Please commit once this is addressed.

(follow-up: ↓ 5 ) 09/02/08 06:40:49 changed by crschmidt

"line 482: OpenLayers.Util.getElement should be used as opposed to document.getElementById"

OpenLayers.Util.getElement is useful where you might have an ID *or* an element, but it introduces additional overhead in a case where you have only an ID. if there is no expectation that the function will be handed anything other than ID, then using Util.getElement is unnecessary (and potentially harmful).

(in reply to: ↑ 4 ) 09/02/08 07:01:12 changed by elemoine

Replying to crschmidt:

"line 482: OpenLayers.Util.getElement should be used as opposed to document.getElementById" OpenLayers.Util.getElement is useful where you might have an ID *or* an element, but it introduces additional overhead in a case where you have only an ID. if there is no expectation that the function will be handed anything other than ID, then using Util.getElement is unnecessary (and potentially harmful).

Oh ok, thanks for jumping in Chris.

(in reply to: ↑ 3 ) 09/02/08 12:13:59 changed by ahocevar

Replying to elemoine:

lib/OpenLayers/Renderer/Elements.js * line 743: isn't "or null if the renderer could not draw all components of the rectangle" missing here too? Maybe not...

No, the rectangle works similar to a point. It has an origin and a width and height, so we can draw either all or nothing.

09/02/08 12:20:33 changed by crschmidt

  • status changed from new to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

r7930 /trunk/openlayers/ (12 files in 7 dirs): New vector rendering for better performance and less renderer specific limitations. r=elemoine (closes #1675, closes #1656, closes #1631, closes #1431, closes #1709)