Make world limits not forced to water level

[Markohs]

ooooh, right, I see when this fails get_tile_width marks 170, and that's rounded from 170,66666666666666666666666666667.

Maybe rearanging this calculations I'll get rid of that error, I'll see what can I do.

Thanks a lot Dwachs, really helpful as usual!!

What I don't know yet is why this doesn't happen on other parts of the code.

[Dwachs]

Did you saw my edit?

[Markohs]

yes, thx

EDIT: If I understand correctly, with your modification, the minimized error it has now it's due to the accumulated error in the modulus in the vertical paiting (the %he_bg), one pixel for every painted tile.

Do you think this error will be acceptable? It's very small...

[Dwachs]

It seems to be just one pixel off, this is acceptable. I do not know, where this difference does come from. Maybe this is caused by some round-off 'bug' in the (original) drawing code.

[Markohs]

It seems to be just one pixel off, this is acceptable. I do not know, where this difference does come from. Maybe this is caused by some round-off 'bug' in the (original) drawing code.

I think it comes from the modulus I apply to the coordinates to get the start of the pattern.

Anyway if the image that's defined it's a multiple of the basic tile dimensions, this error should not show, I think.

Okay, thanks a lot again Dwachs.

[Ters]

My problem was not related. I just had to throw (get_base_tile_raster_width() - IMG_SIZE)/2 into the mix. Reminds me of math back at school, when we sometimes got the right answer, except that it was twice or ten times as big (or small) as the correct answer. So we just multiplied (or divided) by two or ten to get the right answer, without really knowing why. (This wasn't on exams of course. There were no correct answers in the back of the book then.)

[prissi]

Ters, this appears only a certain screen sizes. Markohs could be lucky and this is then zero, if we are talking about the same offset. When there is an uneven number of tiles for the screen, the it was centered. (Actually this code still stems mostly from Hajo ... )

[Ters]

Ters, this appears only a certain screen sizes. Markohs could be lucky and this is then zero, if we are talking about the same offset. When there is an uneven number of tiles for the screen, the it was centered. (Actually this code still stems mostly from Hajo ... )

I'm not sure whether you're referring to the problem Markohs had, or the one I had. My point was that they seemed to be unrelated problems. His problem seems to be related to a single zoom level, while my problem scaled linearly with zoom factor, being only correct at 1:1.

[Markohs]

My problem was not related. I just had to throw (get_base_tile_raster_width() - IMG_SIZE)/2 into the mix. Reminds me of math back at school, when we sometimes got the right answer, except that it was twice or ten times as big (or small) as the correct answer. So we just multiplied (or divided) by two or ten to get the right answer, without really knowing why. (This wasn't on exams of course. There were no correct answers in the back of the book then.)

Well, yes.

At first design I just copied code around and found result was more or less accurate, but needed some adjustments. Multipled, divided, truncated, added 1... Nothing worked perfectly. Then I decided to understand the algorithm and write from scratch. Better result but not perfect either, more adjustments, factors, trying... Then you point me to zeiger and same cycle again hehe.

Until Dwachs came with that!

[Markohs]

State of the patch:

https://dl.dropboxusercontent.com/u/30024783/background-9.patch

I'm substituting PUSH_CLIP by add_poly_clip, Dwachs code, that's used on dinge drawing and should not be affected by this code, that's called in display_boden(), I think that will get rid of clipping corruption.

1) Right now I get background from ground.outside.pak but that restricts max image size to the pak size. I'm thinking in reading it from ground.background.pak , with arbitrary size. Anyone against this?
2) I added all the code to karte_t , but thought on creating simbackground.cc with background_t , do you think it's a good idea or it's ok like this?
3) I cached some calculations in karte_t, to save time when redrawing, I guess that's ok too.

Comments welcome.

[prissi]

Just very short remarks without reading your patch in depth.

add_poly_clip is magnitudes slow than normal clip. Be careful to not become slower than even before when always drawing full screen.

The drawing should go to simview.cc. That should not be hard to do. (Ok, maybe a slight change needed to gui_worldview?)

And finally a remar on your comments:
/**
* Updates cached values required for background display.
*/
void update_background_cached_values(bool changed_zoom=false);

The comment in this case does not explain anything at all. (Same as the comments at the background variable btw.) It just repeats the name.

Same for
@name Background management
*       This (These!) variables are related to the background displaying.
* @note width and height are stripped of their lower 2 bits because routines in simview.cc use divisions by
* 2 and 4 of this value. This causes align problems at zoom level 3 to 4.

But why or related in what way is still completely unclear ...

[Markohs]

Just very short remarks without reading your patch in depth.

add_poly_clip is magnitudes slow than normal clip. Be careful to not become slower than even before when always drawing full screen.

Okay will look into it.

The drawing should go to simview.cc. That should not be hard to do. (Ok, maybe a slight change needed to gui_worldview?)

Well, grund_t also calls this draw background, but ok, will see how can manage it.

And finally a remar on your comments:
/**
* Updates cached values required for background display.
*/
void update_background_cached_values(bool changed_zoom=false);

The comment in this case does not explain anything at all. (Same as the comments at the background variable btw.) It just repeats the name.

Same for
@name Background management
*       This (These!) variables are related to the background displaying.
* @note width and height are stripped of their lower 2 bits because routines in simview.cc use divisions by
* 2 and 4 of this value. This causes align problems at zoom level 3 to 4.

But why or related in what way is still completely unclear ...

Well, I'll take it as a compliment of my function names being well chosed and expressive...


But ok, you are right, I'll add more detail, not so much to turn it to a novel but I'll add more detail.


Thanks for having a look!

[prissi]

Then maybe do the drawing in grund_t, or planquadrat. I would like to avoid another place where drawing happens. The code is alrady quite complex. It may be an ugly thing to say, but maybe really going back to your first optimisation to draw all the background only when something is visible is the better way in terms of complexity versus gain.

[Markohs]

Then maybe do the drawing in grund_t, or planquadrat. I would like to avoid another place where drawing happens. The code is alrady quite complex. It may be an ugly thing to say, but maybe really going back to your first optimisation to draw all the background only when something is visible is the better way in terms of complexity versus gain.

Yep, was thinking that too.

I'll first try another concept, remove background drawing in grund_t (safety borders) and just leave full screen background draw when visible (once), plus modifying the dirty tile concept that's now:

1) mark dirty
2) copy to screen

To:

1) mark dirty
2) copy to screen
3) drawn background on that zone, so next redraw it's ok.

I think it can work, having it a look.

[prissi]

The code has a big problem with tile heights !=16, pakTTD for instance.

EDIT: Also removing the ticker leaves artifacts now.

EDIT2: I changed the code to tilewise drawing only, and removed all the background dirty stuff. Found no artefacts, but please test again.

[Markohs]

dunno if the way you decided to implement this is the best, really, prissi, it's slower than my solution, and it leaves artifacts anyway. It also makes impossible to implement a background image. I don't really like how the code looks right now.

Whoudn't just have been better fixing the border background code that I submited?

It has bugs, too, just raise/lower tiles at the border of the world, and you'll see .



Why do you draw at grundwasser leveloutside the map? Now it can be any height.

Code Select Expand


                else {
                    // outside
                    const sint16 yypos = ypos - tile_raster_scale_y( welt->get_grundwasser()*TILE_HEIGHT_STEP, IMG_SIZE) + (IMG_SIZE*3)/4;
                    display_fillbox_wh_clip( xpos, yypos, IMG_SIZE, IMG_SIZE/4, umgebung_t::background_color, force_dirty );
                }


I might be wrong, but I think you assume the tile will be at water level, and it doesn't really need to be.

[prissi]

Which it is slower. You drew infinite band and cleared all the screen. It would have been fine with one of them. Anyway, your calculation created lots of artifacts with half tile height or pak64. I would be perfectly fine with that too. However, then the fuss in grund_t is not needed at all, so just get rid of this. Shortest and still no big impact I would say.

Without grundwasser, then the top would generate artefacts. Can be removed of course, it was just a continuation of the existing code. Try next submit with the attached pak64 savegame. It has all artefact creating stuff I could think off.

[TurfIt]

The ingame cursor can get stuck on the map when you rapidly move the real cursor off the map onto the background region. If you then activate the selected tool, it does work at this stuck position.

[Markohs]

 mmm... I'm a big lost now.

So you removed the background drawing in grund_t but the full background redraw and dirty background marking mechanism is preserved?

Well, if whould be great is that was enough!

But looking at your savegame, a few extra changes are needed somewere, because:



The one in the "follow" screen just needs that area being flushed with background, and the one in the southern border, a background_mark_dirty on modification of a south or east grid position.

I can fix them, if you want it, or you can fix yourself, as you wish.

You are right, my code in grund_t was not necessary if this works.

The ingame cursor can get stuck on the map when you rapidly move the real cursor off the map onto the background region. If you then activate the selected tool, it does work at this stuck position.

That can be fixed too, if when toggling the tool, we check if it's a grid or tile tool, and if the last known position is invalid, we move it to the nearedt allowable position. It can be fixed.

[Markohs]

Well, nevermind, I fixed the bugs and commited to svn @6484, I hope you don't mind.

I define karte_ansicht_t::display_background as you suggested some posts ago to move display code to the view. Right now it's just a call to a fillbox but will be more complex on my next patch.

EDIT: Regarding turfIt menctioned bug, this solves the problem:

https://dl.dropboxusercontent.com/u/30024783/background_prissi.patch

But I'm unsude if applying it or nowt because it doesn't solve another problem I detected, and that's after a grid node it's raised on the edge of the map, the cursor (zeiger), it's not updated of it's z-coord before the frame display comes, so you can see the arrow on the old z-position. This doesn't happen just on the border, it happens in the wole map, allways has been that way, it's just conflicting with prissi's aproach to this, because the zeiger was overdrawn before, each frame. But because it looks somewhat network related, I didn't dared to change further.



I can try to do it anyway (modifying zeiger after the tool has done the "work", comments welcome.

[Dwachs]

your patch does not look like it could ruin network mode

[Markohs]

yep, up to that I know, but I'll need to move zeiger after werkzeugt_t::work (or whatever it's spelled, sometimes german drives me mad ). Moving zeiger won't break networkmode? I'll need to increment/decrement it's z value.

[Dwachs]

Moving zeiger won't break networkmode?

No, its position is taken during the reaction on mouseclick (somewhere in simworld.cc, interactive_event ?).

[Markohs]

Okay, third version of the patch to fix the current artifacts:

https://dl.dropboxusercontent.com/u/30024783/background_prissi-3.patch

Lots of changes, lower/raise tool's position now updates the cursor pointer immediately after using. Not only on world borders, on the whole map. Now when you raise/lower, the pointer will move instantly to the vertex that will be affected if you use the tool again. It was a bit strange how it worked before, because if you just re-clicked without moving the mouse pointer, you could end raising a vertex that was not expected (the cursor was not over it). And as a side-effect, now the pointer won't be drawn over background.

Looks like the stripes painted on grund_t cleaned more than we thought (at least me), this still fails (using a construction tool too near of the border):



I guess at this point, forcing a background redraw after cursor moves from a border position, it's mandatory too.

I know I changed quite a lot of things but believe me, there was not better way of doing it, (or I coudn't find it).

Comments welcome, as allways.

EDIT: the bug was easy to fix, https://dl.dropboxusercontent.com/u/30024783/background_prissi-4.patch , anyone finds more artifacts?

[prissi]

Sorry, I just submitted five lines in zeiger_t, which took care of the background. But those are obviously not updating the cursor position.

[TurfIt]

Shadows should be clipped to the tile edge rather than being cast into the void.. IMHO that looks just plain wrong.

I can still make the cursor get stuck in the middle of the map by rapidly moving it from a ground tile to the background. Clicking to activate a tool then executes it once before the cursor disappears entirely.
If I move the cursor off map slow, it gets stuck along the edge. Then clicking with a tool executes every click. Ingame cursor should become invalid whenever the real cursor is off the map IMO, and prevent map altering tools from operating.

[Markohs]

@prissi: ah, ok, then my modification to zeiger.cc is not needed, removed it, your modification does the same.
@TurfIt:
  about shadows: I fear clipping just the shadow it's not possible, after all it's just part of the image itself, how do you distinguish wich part of the image is just shadow and wich is the actual tree spanning to the top of the tile? No idea, I don't really know much of how dings are drawn still (we can maybe just implement that on baum.cc), just know a bit of the landscape code. Anyone has any idea on how to implement that without consuming too much CPU? It's a similar case than someone mentioned here some weeks ago, that in some pak some trees have offsets so big that span to the tile in the south, in that case trees float in space.

  about the cursor:

I think this fixed all the issues:

Code Select Expand


void karte_t::move_cursor(const event_t *ev)
{
   if(!zeiger) {
      // No cursor to move, exit
      return;
...
   const koord3d pos = get_new_cursor_position(ev, wkz->is_grid_tool());

   if( pos == koord3d::invalid ) {
      zeiger->change_pos(pos);
      return;
   }


updated patch:

https://dl.dropboxusercontent.com/u/30024783/background_prissi-6.patch

I think I can just commit it tomorrow, unless someone finds a bug or has more comments.

[TurfIt]

re shadows: I thought they they were drawn seperately, like back/front images. Obviously not possibly when they are one image. doh.

[greenling]

Wooh those projekt goes forward that i me wonder.

[prissi]

If the background color is set to black, the shadow problem is solved immediately...

I am not sure that a valid zeiger pos is not assumed at more positions. Lets try this out.

By the way, I just remembered, why the cursor pos was not updated. That stemmed once from the massive flattening options by keeping the old coordinates. But it might well be already obsolete. But dragging a plateau should still work, or?

EDIT: Why the extra code in raise_to? This routine is already relatively slow for higher hills. Given also the low frequency for the raising or lowering of tiles by AI or user, I would just trigger the whole background dirty for any such operation.

[jamespetts]

If the background color is set to black, the shadow problem is solved immediately...

I am definitely in favour of this - the black background looks rather better than the grey version that I have seen.

[Markohs]

If the background color is set to black, the shadow problem is solved immediately...

About the default color in background: it's already able to be modified in each pak's simuconf.tab, or by each player manually ofc.

Dunno, I personally like this gray and don't mind the shadow errors. If I had to decide a color myself I'd use a super-bright disgusting yellow color to force pak creators to decide the color that suits better for their pak.

If you want to change it, go ahead, I don't mind.

I am not sure that a valid zeiger pos is not assumed at more positions. Lets try this out.

I just commited the patch, with 2 extra changes that set cursor to koord3d::invalid in certain circunstances. Coudn't find any bug.

By the way, I just remembered, why the cursor pos was not updated. That stemmed once from the massive flattening options by keeping the old coordinates. But it might well be already obsolete. But dragging a plateau should still work, or?

You mean clicking a raise_lower tool and without unclicking drag the mouse to make flat terrains? If you refer to this, yes it's working right now, tested yesterday and just tested again. If you don't mean that I don't really understand what you said.

EDIT: Why the extra code in raise_to? This routine is already relatively slow for higher hills. Given also the low frequency for the raising or lowering of tiles by AI or user, I would just trigger the whole background dirty for any such operation.

mmm... you are right, changing it. It was because of its recursive nature, raising (60,60) in a 64x64 map could potentially end raising border tiles, and wanted to just mark dirty then. But since this is a user-triggered acction, won't affect performance marking background dirty each time a player uses the tool.

Changed, all commited to @6495

@greenling: I don't really understand what you said, sorry.

[An_dz]

@greenling: I don't really understand what you said, sorry.

He tried to say:
"Wooh This project got more development than I thought so."

[TurfIt]

Detection of the mouse cursor in the background region only works when the world meets the background at sealevel. If the ground is higher, there's a misalignment between the mouse cursor and ingame cursor at the boundary.

[Markohs]

mmm... can't reproduce what you say, TurfIt, it works good here...

Wich pak you use and in wich border did you try? east/south? Or north/west? Wich tool you were using? I tried pak64 and pak128, with inspect and raise tool, and on all corners and all looked good to me... :?

Dunno if I understand you fully, on raise/lower tool it's normal to get a misalign because it will move to the closest vertex, and on tile tools, I allways got the one under the mouse pointer. I tested Windows GDI version btw. might it be the default system mouse pointer graphic your system uses it's not aligned to the one the system sends to the application?