OpenCPN Beta test 4.1.1108 Release

[bdbcat]

Hakan...

Re;
"gave another effect in none quilt mode when close to a F part, see shoot."

This looks like expected behaviour. The F-scale cells are not continuous in this area. So the world background chart shows in the "gaps".

Or am I missing the point?

Thanks
Dave

[Hakan]

Dave
No, you're probably not missing any points. It could be I haven't used non quilt mode here before, so my words; "gave another..." is not quit relevant I suppose.

In quilt mode it's now much better, not quit as 4.0 but close to. I hope the code changes since then has increased other performances and today's patch didn't forced new problem.
Many thanks!

Håkan

[transmitterdan]

I believe it was Donald Knuth that said:

"The real problem is that programmers have spent far too much time worrying about efficiency in the wrong places and at the wrong times; premature optimization is the root of all evil (or at least most of it) in programming."

In other words, get it right before getting it fast. And when we do make efficiency improvements it should be only where it matters and not at the expense of getting it right. But everyone knows that already.

[Hakan]

Dan
That philosophic way of expression could have been my own if only have had enough skills in English and the mind to put thoughts into word.
This forum is a wonderful collection of skills, cultures and consciousness.
Cheers!
Håkan

[transmitterdan]

Knuth came up with that over 40 years ago. I have used his famous line many times. So much so that some people I work with think I invented the words.

[bdbcat]

Hajkan...

"not quit as 4.0 but close to"....

Screenshots, please?

Thanks
Dave

[Hakan]

Dave
First it was easy to answer, see left picture, ver 4.0 to the left.
But your undertone(correct word?) of suspicion made me check carefully. Now my 4.0 instance has a detail level 2 and when I changed to 4, same as 4.1.1108, the result was as of right picture. Equal!
I'm sorry if I have had a bad undertone without a serious intention.
You know I really appreciate all your work and skills.

Thanks
Håkan

[seandepagnier]

Originally Posted by bdbcat

Hakan, did-g, Sean, etc...


Various Regressions:

1. Hakan's jumping scale problem is provisionally resolved. The problem only occurs in OpnGL, without FBO enabled, like most MSW/Intel configurations. You can induce it in linux by forcing FBO's off.

Nothing wrong with the basic cm93 quilting logic. But the S57 renderer must obey exactly the specified render region for the logic to work


The location and scale specified in Hakan's test is especially complex, to wit:

a. The base scale requested by zoom and detail slider is F. There is a tiny sliver of F scale cell available at the left side of the display. In full-screen quilting mode, this sliver controls the cm93 quilt compositor. So the little F scale piece gets rendered.
b. The main area of the screen is filled with E scale cell, since there is no F-scale available here.
c. There is another tiny sliver on the right that is not covered by F, E or D. So the logic retires to C scale. And the C scale render sometimes gets it wrong, writing over the entire area by not honoring the region logic.

The C scale should render first before E or F, so I don't understand why it can write over the area.

I made a provisional fix in s57chart.cpp, applied only to cm93. May be slower for complex cm93 quilting situations.
@Sean, please review and comment.

It's a bit more expensive to set a complex clipping region than a basic rectangle. Especially considering we can use glScissors which is nearly for free. I eliminated clipping regions for the world background chart for example by ensuring it is rendered before other charts rather than after.

I had assumed that in quilted mode it would render the charts in the correct order such that the highest detail chart is rendered last so in theory clipping regions are not even required at all for vector charts. Now, maybe, the cm93 charts are technically invalid and produce geometry outside their prescribed boundaries, or maybe somehow they got rendered in the wrong order.

Now, I had imagined that for raster charts we could pre-compute coordinates which honor the chart outlines. These cached quilted coordinates would allow rendering without clipping regions. I didn't get to this yet. This would usually be faster too because it would push fewer fragments, and avoid depth/stencil tests.

I must had mistakenly assumed that the chart would not try to render outside where it should.. but obviously it does somehow in quilted mode. I think this is maybe a bug in the charts, or we should detect and correct this once so that we don't have to set the more expensive clipping region every frame.

Also, the code only caches the last clip rect which has to be restored in some cases for depth mode (RenderToGLAP) so this must be dealt with as well.

2. did-g Issue 1:
Problem identified as failure of region logic as cm93 cells are loaded.

@Sean: cm93chart::IsPointInLoadedM_COVR() does not produce the same result for the two available conditional paths. Try it with VP centered at (49 08.5000 N, 001 59.0000 W), single chart mode, scale set so as to load D-scale chart. The old way works, the new way does not, at least in single-chart mode.

I fixed this provisionally. Probably slower than the new llRegion method, but works.

Yes, it is a bit slower so I am investigating. More importantly it should work so it might explain other errors.

Associated issue: Missing NODTA areas:
@didier, what you are seeing is the water area of the GSHHS world chart, rendered in error behind the cm93 cell, and thus showing through in the NODTA areas. Verify by setting Display category to Mariners Standard, and clearing all Feature types. You will see the same color blue on the right half of the display.

Not sure what is causing this, may be related to LON0. I'll investigate.
@Sean, please take a look.

Dave

It may draw the world background chart first, but it should should draw the NODTA area. Eventually I intended to cache the nodta area coordinates, and in theory (although very complicated) they could even somehow be reduced to a smaller area when possible. It would be nice if they were just part of the vector chart's normal geometry instead.

[seandepagnier]

Dave,

The "fix" for contains is probably not right. I can be sure it will not give the right result for longitudes between -180 and -160 as the old regions go from -160 to 200 where the LLRegion is -180 to 180.

If I could somehow find a case where LLRegion::Contains gives a different result when the longitude is not in this range then I could debug it, but so far I cannot find such a case anywhere, not even at (49 08.5000 N, 001 59.0000 W)


Also I wanted to add, that maybe vector charts always will need region based clipping regions to prevent things like text or light sectors from going outside their area.. but I am not sure if it is actually wrong for this to happen or not.

[did-g]

Sean...

Originally Posted by boat_alexandra

Dave,

The "fix" for contains is probably not right. I can be sure it will not give the right result for longitudes between -180 and -160 as the old regions go from -160 to 200 where the LLRegion is -180 to 180.

If I could somehow find a case where LLRegion::Contains gives a different result when the longitude is not in this range then I could debug it, but so far I cannot find such a case anywhere, not even at (49 08.5000 N, 001 59.0000 W)


.

Is the issue in Contains?

the first graph is the new LLRegion m_region (closing segment of each polygon not drawn).

The second m_pcovr_array_loaded
m_region should be the union of m_pcovr_array_loaded arrays, right?

[bdbcat]

Hakan...

Thanks for double checking against O4. No offense taken here. Just want to understand the beast, in all its fractious moods.

Dave

[bdbcat]

Sean...

1. re:
"The C scale should render first before E or F, so I don't understand why it can write over the area."

This is the way the Quilt compositor works, but is not the way the cm93 compositor works, currently. cm93 draws the patches in decreasing scale order. So honoring the clip region is critical. I agree that the scissors is very fast. But we cannot use scissors on MSW Intel platforms, our largest user base, since scissors don't seem to work right with depth buffer clipping. And on this platform, we usually have to use depth buffer clipping. So complex region clipping is not much slower, in most cases.

2.
"I must had mistakenly assumed that the chart would not try to render outside where it should.. but obviously it does somehow in quilted mode. I think this is maybe a bug in the charts, or we should detect and correct this once so that we don't have to set the more expensive clipping region every frame."

The object inclusion test is done on the (possibly clipped) VP in s52plib::ObjectRenderCheckPos(). The test is done only on the VP's bounding box. This can and does easily include objects outside of the clip region, but inside the clip rectangle. Technically, this is an error. But we decided very early on that object testing against a region was too expensive to be applied on a per-object/per-frame basis, especially for area-type objects. So we depend on a proper clip region in GL space.

And there are other reasons, as you later note.

3.
re: cm93chart::IsPointInLoadedM_COVR()
did-g has it analyzed right. Somehow the LLRegion.Union() is not producing the correct result. See his perfectly instructive picture.

4.
re:
"Eventually I intended to cache the nodta area coordinates, and in theory (although very complicated) they could even somehow be reduced to a smaller area when possible. It would be nice if they were just part of the vector chart's normal geometry instead."

I'm not sure this is a win. A rectangle fill of a fixed colour is just about the fastest raster op there is, so decomposing and identifying NODTA areas as separate objects seems to me to be a waste of cycles. Also, this is the way S57 charts are meant to be rendered. Anything that is not actually rendered is, by definition, a NODTA area. And so the pre-painted NODTA color shows through the holes.
But maybe I'm not following your logic.


Dave

[.Paul.]

Originally Posted by bdbcat

Hakan, did-g, Sean, etc...


Various Regressions:

1. Hakan's jumping scale problem is provisionally resolved. The problem only occurs in OpnGL, without FBO enabled, like most MSW/Intel configurations. You can induce it in linux by forcing FBO's off.

...

Dave

I built O from the source Dave revised on 15 Nov, and I have run Hakan's VDR several times in this version.

It renders the quilted chart detail much better. Also, the small region south of St Dagholmen (an island) from 57 59.8N 11 46.8E and SE from this lat/lon now has spot soundings and depth contours. My previous build from 14 Nov only had blue (shallow water) there. All of the water areas appear to be correctly rendered.

A few place name text labels disappear once in a while when the chart canvas is redrawn. The next redraw brings the labels back.

The C scale chart that fills the right side of my screen has the buff color of land with one symbol for a Building, religous [sic] (BUIREL), as shown below.



From time to time this region is displayed in gray, as below.



It stays gray for only a short time, then the buff land and BUIREL symbol return as the charts move under the red boat.

All in all, it is an improvement.

Thanks,
Paul

[bdbcat]

Sean...

Here is a simple fail case to investigate:

LLRegion::::InitPoints( size_t n, const double *points)

Fails for a set of points which have large positive longitude, say 358 deg, as at or near the Jersey Island point under discussion.

It fails because

void LLRegion::AdjustLongitude()
{
.
.
.
leads to
resolved.Subtract(clip);

leads to
NoIntersection(const LLRegion& region)


leads to GetBox()
which returns the wrong box phase.

The result finally is that InitPoints() ends up producing an empty region.

And that is why LLRegion.Union() seems to fail. It is union'ing an empty region.

And so the GSHHS chart background is painted in the wrong place.

Resolve this simple case, and all will be well, or at least a step in the right direction, I think.

Dave

[seandepagnier]

First off, I fixed the problem with the lines around africa and not rendering properly in non-opengl mode... It was an issue when the viewport crosses both 0 and 180 at the same time.

Originally Posted by bdbcat

Sean...

1. re:
"The C scale should render first before E or F, so I don't understand why it can write over the area."

This is the way the Quilt compositor works, but is not the way the cm93 compositor works, currently. cm93 draws the patches in decreasing scale order. So honoring the clip region is critical.

Ok... so can we just rearrange it to do this?

I agree that the scissors is very fast. But we cannot use scissors on MSW Intel platforms, our largest user base, since scissors don't seem to work right with depth buffer clipping.

I really need to play with it some more to verify this. My upgraded intel driver on linux is much better. Also, all the android and single board arm platforms probably can use scissors. Maybe I should just add logic to use scissors in the cases where the region fits as a rectangle as this is often the case. In any case I can see there is not an easy way to avoid the clip region for now.

2.

The object inclusion test is done on the (possibly clipped) VP in s52plib::ObjectRenderCheckPos(). The test is done only on the VP's bounding box. This can and does easily include objects outside of the clip region, but inside the clip rectangle. Technically, this is an error. But we decided very early on that object testing against a region was too expensive to be applied on a per-object/per-frame basis, especially for area-type objects. So we depend on a proper clip region in GL space.

It is a hard call, because in some cases, the region test is a lot cheaper than drawing the object. But the other big problem is the cases where the object is partially in the region and partially outside, and so should be partially rendered which is very complicated to do without a clipping region. Maybe it's just me, but I tend to think text and light sectors at least should be allowed to spill outside the region.

3.
re: cm93chart::IsPointInLoadedM_COVR()
did-g has it analyzed right. Somehow the LLRegion.Union() is not producing the correct result. See his perfectly instructive picture.

I found this as well yesterday... I think the problem is when two adjacent regions have identical segments and points.. so zero vectors get created. I will investigate now that I can reproduce it.

4.
re:
I'm not sure this is a win. A rectangle fill of a fixed colour is just about the fastest raster op there is, so decomposing and identifying NODTA areas as separate objects seems to me to be a waste of cycles. Also, this is the way S57 charts are meant to be rendered. Anything that is not actually rendered is, by definition, a NODTA area. And so the pre-painted NODTA color shows through the holes.
But maybe I'm not following your logic.


Dave

You are right, it's probably not worth bothering with. Maybe only in the case that we can determine that there are no areas where NODTA is visible so we can avoid drawing it completely, but for now we don't need to worry. I should cache the glTess results for this area so it isn't computed each frame.

Originally Posted by bdbcat

Sean...

Here is a simple fail case to investigate:

LLRegion::::InitPoints( size_t n, const double *points)

Fails for a set of points which have large positive longitude, say 358 deg, as at or near the Jersey Island point under discussion.

It fails because

void LLRegion::AdjustLongitude()
{
.
.
.
leads to
resolved.Subtract(clip);

leads to
NoIntersection(const LLRegion& region)


leads to GetBox()
which returns the wrong box phase.

The result finally is that InitPoints() ends up producing an empty region.

And that is why LLRegion.Union() seems to fail. It is union'ing an empty region.

And so the GSHHS chart background is painted in the wrong place.

Resolve this simple case, and all will be well, or at least a step in the right direction, I think.

Dave

I think the problem is the shortcut logic using NoIntersection is not actually valid before the longitudes are resolved in this case... So it could be bypassed in this case.

The reason it is not valid is I never intended to pass points that lie completely outside of -180 to 180. Really these points should be resolved before they reach InitPoints.