News:

The Forum Rules and Guidelines
Our forum has Rules and Guidelines. Please, be kind and read them ;).

Make world limits not forced to water level

Started by Markohs, November 16, 2012, 01:39:10 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

TurfIt

What do you mean by setting background dirty whenever a tooltip is drawn? Forcing a complete redraw every frame when a tooltip is onscreen?

IMHO, redrawing the whole screen should be the absolute last resort. Doing so is the cause of the huge slowdown with the use_hw doublebuffering when no hardware support is exposed by the driver. The dirty tile update mechanism is vital to good performance, and needs to be extended to handle this new background mechanism IMO...

p.s.  if r6451 is meant to fix this, the game crashes immediately upon choosing a pak.

Markohs

#176
wl->set_background_dirty(); need to be protected as

if(wl)
wl->set_dirty();

Because wl can be NULL at start


EDIT: Also


void win_poll_event(event_t* const ev)
{
display_poll_event(ev);
// main window resized
if(ev->ev_class==EVENT_SYSTEM  &&  ev->ev_code==SYSTEM_RESIZE) {
// main window resized
simgraph_resize( ev->mx, ev->my );
ticker::redraw_ticker();
wl->set_background_dirty();
ev->ev_class = EVENT_NONE;
}
}



You changed wl->set_dirty() to set_background_dirty(), I think this is not a good change, wl->set_dirty already forces a background_dirty() too.



void karte_ansicht_t::display(bool force_dirty)
...

// redraw everything?
force_dirty = force_dirty || welt->is_dirty();
welt->unset_dirty();
if(force_dirty) {
mark_rect_dirty_wc( 0, 0, display_get_width(), display_get_height() );
welt->set_background_dirty();
force_dirty = false;
}



I'll revert that change if you don't mind, I think it's incorrect.


I commited this as 6452, segfaults should have disappeared.

Markohs

Quote from: Dwachs on April 13, 2013, 03:06:02 PM
There are still problems with the gray background not set dirty: if tooltips are over the background, they leave traces. Not sure how to solve this. Of course, we could set the background dirty whenever a tooltip is drawn. Would this be a performance bottleneck?

Mmm... having a look to this now and I can't reproduce it.

I paint the "safety" border background with the dirty flag *OFF*, I thought that way only the really needed part (overwritten) will be flushed to screen, saving some performance, notice in grund_t::display_border the calls to display_fillbox_wh_clip have dirty set to false.

wich backend experiences the problems? I use Windows GDI (2 threads) and all shows perfect, when scrolling and when not.


This shows good:
 



Markohs

Quote from: TurfIt on April 13, 2013, 07:15:35 PM
What do you mean by setting background dirty whenever a tooltip is drawn? Forcing a complete redraw every frame when a tooltip is onscreen?

IMHO, redrawing the whole screen should be the absolute last resort. Doing so is the cause of the huge slowdown with the use_hw doublebuffering when no hardware support is exposed by the driver.

Agree, that's why I made the optimization patch.

Quote
The dirty tile update mechanism is vital to good performance, and needs to be extended to handle this new background mechanism IMO...

p.s.  if r6451 is meant to fix this, the game crashes immediately upon choosing a pak.


I agree, but doesn't the current patch handles this already? What do you suggest?

TurfIt

I'm not really sure what the current state handles. Unfortunately I didn't have a chance to really follow development of this. It just seems like there's an awful lot of set_background_dirty() calls with the resulting fullscreen flush required. Are individual rectangles in the background being marked dirty, or just the whole thing?

Also, is the Simcity 4 view still possible? i.e. with the border showing the underground gradient. I somewhat like that better than the floating world.  And, is it possible to restore the force to waterlevel function (as an option)? Along with the old tiled display for out of bounds? I also quite like the 'old' way with the whole world shown and surrounded by dangerous shoals... beware, here be dragons!

Markohs

Quote from: TurfIt on April 13, 2013, 11:33:53 PM
I'm not really sure what the current state handles. Unfortunately I didn't have a chance to really follow development of this. It just seems like there's an awful lot of set_background_dirty() calls with the resulting fullscreen flush required. Are individual rectangles in the background being marked dirty, or just the whole thing?

set_background_dirty()  forces  a full screen background redraw *ONLY* if it's going to be visible (the full screen is covered by tiles). That was the optimization. So, following a train (for example) won't flush screen if it's not really needed.

This background redraw *DOESN'T* mark the screen dirty. It's the world that marks screen dirty, as it was before, *BUT* all border tiles are marked as dirty each frame. So those tiles allways flush their section of screen.

But it's not the whole screen.

Quote from: TurfIt on April 13, 2013, 11:33:53 PM
Also, is the Simcity 4 view still possible? i.e. with the border showing the underground gradient. I somewhat like that better than the floating world.  And, is it possible to restore the force to waterlevel function (as an option)? Along with the old tiled display for out of bounds? I also quite like the 'old' way with the whole world shown and surrounded by dangerous shoals... beware, here be dragons!

Yes, all the posibilities you menction are possible and will be implemented. I just want to make sure what's already in trunk is stable enough to keep adding things. :)

TurfIt

Hmmm. I really need to look more into the dirty tile stuff before I can comment more...

As for the tooltip artifacts, I can duplicate that - SDL backend didn't try others. Note tooltips, not labels as shown in your screenshots.

Markohs

oooh I see, my bad, thinking how to deal with this then, there must be a solution to it. Toggling background redraw each frame when a tooltip is acive can be a solution (shouldn't be so horrible) but I'll search for a more elegant one.

TurfIt

The tooltips appear to be marking the dirty tiles correctly, the background image doesn't appear to be redrawing into those dirty sections.  I trust you've tried DEBUG_FLUSH_BUFFER? It's quite illuminating! (and shows some strange lines on the background...)

Markohs

#184
Thank you for the DEBUG_FLUSH_BUFFER, prissi pointed me to it time ago and I forgot it existed. Testing it now, but setting a zone dirty won't make it redraw background there, that's handled separately. I see the problem now, working in a fix. Thx. :) 

EDIT:
To force a background redraw welt->set_dirty() or welt->set_background_dirty() do the job. I also have implemented a karte_t::draw_background(x,y,xx,yy) but it's not submited to trunk yet because it's part of the next planned patch for pixmap background. But maybe it will be needed to fix this.

Anyway if you want to code something or change it somehow, ofc you can do it. I'll go sleep now, will look into this tomorrow. ;)

TurfIt

No, setting it dirty won't redraw the background. I think that's what I was getting at earlier - it should! And only redraw the section that's dirty instead of forcing a full screen update. The same applies with the window moving/resizing setting the background dirty, they also correctly set the dirty tiles and should just trigger background redraw of the dirty tile area rather than the whole screen.

Anyways, have a good night! I'm sure with the relative timezones, you'll have this all fixed before I even get up.  ;D

Max-Max

I updated the code from Trunk today and noticed some trees hanging outside the map...
Well, maybe this is PAK96.comic issue only?
- My code doesn't have bugs. It develops random features...

Markohs

Yes, it's a bug on the PAK. That tree can be floating on top of water too, its offset is too big.

Max-Max

Alright then :P

Your patch is coming along really nice... :)
- My code doesn't have bugs. It develops random features...

Markohs

Quote from: Max-Max on April 15, 2013, 12:46:50 AM
Your patch is coming along really nice... :)

Thx Max :)

Quote from: TurfIt on April 14, 2013, 01:11:37 AM
No, setting it dirty won't redraw the background. I think that's what I was getting at earlier - it should! And only redraw the section that's dirty instead of forcing a full screen update. The same applies with the window moving/resizing setting the background dirty, they also correctly set the dirty tiles and should just trigger background redraw of the dirty tile area rather than the whole screen.

I wish it was so easy as that, that's not possible because when a zone is marked as dirty is *AFTER* it has been already drawn, so I can't draw background there. The only solution that comes to my mind is drawing background *ON NEXT FRAME* there.

It's not a bad idea, and it might work, wl->set_dirty() draws full screen with background and individual set_dirty() only that zone, but on next frame. I'll think on this.

Quote from: TurfIt on April 14, 2013, 01:11:37 AM
Anyways, have a good night! I'm sure with the relative timezones, you'll have this all fixed before I even get up.  ;D

Not really, had a terrible headache yesterday (and not from hangover, I wish. :) )

Markohs

Fixed @64456 .

I forced the check to redraw background on tooltip, Dwachs solution. Shoudn't impact performance since it will only redraw when background is visible, and most players don't look at borders while playing.

To implement this optimally I'd need to keep dirty background tiles structures, like current implementation of dirty tiles. That implies more code more bugs and extra maintenance. I didn't discard yet toggling a background redraw in that precise area after it's flushed to the framebuffer, even I suspect it whould cause artifacts specially in threaded mode.

But current implementation works at acceptable performance, it's simple and clean, and doesn't leave artifacts.

Since future improvements (like OpenGL or SDL2) can make future screen clear/redraw trivial in performance, I think this should be enough.

Markohs

After a formula nightmare I found the way to align background with the camera. I implemented a background of arbitrary dimensions, so, the artist can choose to make a pattern that will align to world limits or not (If he respects the proportion between height and width of the basic tile or not). The greater the image, the greater the performace the code will have.


Just some questions:


I see makeobj supports images up to 256, it's possible for any pak to include a image of arbitrary dimensions? I made my tests with a 128x128 image:





Another issue: PUSH_CLIP corrupts the clipping when in multithreaded code, I think. I tried to protect my new function with a mutex, but corruption keeps coming (only when simple_draw gets activated). I guess I'll have to protect with a lock PUSH_CLIP itself (with exit POP_CLIP ofc). Any ideas?


EDIT: I made some on-screen debug console to help me debug this, do you want me to polish it a bit so we all developers can use it? We can activate it just when DEBUG is defined.

Ters

Is the formula you mention the formula for finding the screen position of a tile? I've been struggling with that too, but have yet to get it to match up properly vertically when zooming. The formula is from "zeiger" drawing in simview.cc, but still.

Markohs

Yes, all those screen to map and map to screen calculations.

One day we should document this properly because those calculations are splattered around the code in different aproaches and maybe making them as inline functions will improve the usability and readibility of the code by huge amounts.

I have identified:

screen -> tile coords:

karte_ansicht_t::display
karte_ansicht_t::display_region
karte_t::get_ground_on_screen_coordinate (this one I just modularized because it was inside karte_t::move_cursor (move_zeiger before)
karte_t::draw_background (my new function, still not in trunk)

tile -> screen coords

zeiger code in karte_ansicht_t::display as you mentioned, plus the loop on ground tiles.

Found quite ilustrating the old comment

   /*
   * berechnung der basis feldkoordinaten in i und j
   * this would calculate raster i,j koordinates if there was no height
   *  die formeln stehen hier zur erinnerung wie sie in der urform aussehen

   int base_i = (screen_x+screen_y)/2;
   int base_j = (screen_y-screen_x)/2;

   int raster_base_i = (int)floor(base_i / 16.0);
   int raster_base_j = (int)floor(base_j / 16.0);

That is the base formula used everywere before passing for the programmers that optimized it in the past. The 16.00 I'm not sure but maybe original simutrans only suported 16 pixel width images.


There is also all the logic related to height of the grid/tiles. Those offsets can be resolved with something similar to:



const int hgt_step = tile_raster_scale_y( hgt * TILE_HEIGHT_STEP, raster_tile_width);


About aligning in different zoom levels, I'm having exactly the same problem, I wanted to end investigating this tonight, but I higly suspect the calculations have to have in mind all offsets ( ij_off and fine offsets x_off and y_off) are relative to the CENTER of the screen, so you need to take that into account.

I think the key is expressed here in ansich::display:


   const int i_off = welt->get_world_position().x - disp_width/(2*IMG_SIZE) - disp_real_height/IMG_SIZE;
   const int j_off = welt->get_world_position().y + disp_width/(2*IMG_SIZE) - disp_real_height/IMG_SIZE;
   const int const_x_off = welt->get_x_off();
   const int const_y_off = welt->get_y_off();


That moves offsets to the center of screen I think.

prissi


Dwachs

Quote from: Markohs on April 16, 2013, 05:59:34 PM
Another issue: PUSH_CLIP corrupts the clipping when in multithreaded code, I think.
Thats true. Where is it used in the display code?

One place I remember: vertical clipping to prevent houses, trees shine through bridges (simplan.cc somewhere). But this needs a fix, as it also clips airplanes flying over them.
Parsley, sage, rosemary, and maggikraut.

Markohs

 Bogh, having trouble having the grid aligned on all zoom levels.

It's strange but on all zoom levels the camera moves sightly in some directions, that's a bit weird because on all games I remember zoom is done either on the center of screen or where the mouse is poiting.

Whould be great deciphering how it exactly determines wich is the exact coordinate that will be the center of the image, and how does that change with zooming.

One thing that confuses me is ij_off, not pointing to the real center tile on screen (or a corner) but somewere near it (sometimes 2 or 3 tiles away from the real center of screen, the fine offsets don't compensate this).

Still working on this, I guess there is something I didn't understand yet.

Ters

Using the zeiger code, I've got my OpenGL rendered surface tile grid aligned with the normally rendered map horizontally, but not vertically. The vertical thing might have to do with the height scaling, but most afternoons after work, I don't feel like doing more coding that day.

Markohs

same problem here,  doesn't match vertically. looking at zeiger atm

Markohs

Oh, it actually worked!! I was not using view_ij_off, just ij_off! zeiger code was very useful.

Thanks!

I'd like this camera position simplified some day if possible, and zooming always to the center of screen. Anyway just because I got too many projects atm, I'll let it be this way, for now.



I suspect I'm not the only one that finds current zooming a bit weird. :)


I'm pretty sure your vertical align problem comes from height management, but check you use get_view_ij_offset and get_world_position, both are needed.

Yona-TYT


Quote from: Markohs on April 17, 2013, 11:32:44 PM


I suspect I'm not the only one that finds current zooming a bit weird. :)






I would like to be able to zoom in cursor position :thumbsup:

IgorEliezer

#201
Quote from: Yona-TYT on April 18, 2013, 01:08:01 AMI would like to be able to zoom in cursor position :thumbsup:
Markohs, no escaping. ;)

Ters

I'm using get_view_ij_offset. The strange thing is that my misalignment is less than a tile, so I don't think the error is in koord values. The misalignment also scales proportionally to the zoom, it doesn't jump randomly around like it did last week.

Fabio

Zooming with mouse wheel should center on cursor, IMHO.

Markohs

Yep, I agree, but the amount of code needed to change that is quite big (and possible bugs) . We'll try to adress that sometime after this patch.

Quote from: Ters on April 18, 2013, 04:30:38 AM
The strange thing is that my misalignment is less than a tile, so I don't think the error is in koord values. The misalignment also scales proportionally to the zoom, it doesn't jump randomly around like it did last week.

Maybe you are scaling y_off x_off fine offsets? That's expressed in just on screen pixels and should not need to be multiplied by anything.

*I THINK* ;)

Fabio

A kludge could be when zooming center the view to cursor then zoom...

Ters

The code I use is this:

const koord cpos = -welt->get_world_position() - welt->get_view_ij_offset();
const sint16 cx = (cpos.x-cpos.y)*(IMG_SIZE/2) + const_x_off + (IMG_SIZE/2);
const sint16 cy = (cpos.x+cpos.y)*(IMG_SIZE/4) + ((display_get_width()/IMG_SIZE)&1)*(IMG_SIZE/4) + const_y_off;

The part with IMG_SIZE/2 is to get the upper point of the tile, not the upper-left corner of the screen-aligned rectangle surrounding it.

Markohs

#207
Having another look to this, I still have the bug in some zoom levels, my code is equivalent to yours except with the IMG_SIZE/2.

Anyway it should not be a problem because we are experiencing vertical align problems.


I just get align problem on one zoom level, of 1/4 of width difference, rest looks ok.


   const int wi_tile = (int) get_tile_raster_width();
   const int he_tile = (int) wi_tile/2;

   const int wi_bg = (int) get_tile_raster_width();
   const int he_bg = (int) get_tile_raster_width();


   const koord diff = koord(0,0) - get_world_position() - get_view_ij_offset();
   const sint16 x = (diff.x-diff.y)*(wi_tile/2) + x_off;
   const sint16 y = (diff.x+diff.y)*(wi_tile/4) + ((display_get_width()/wi_tile)&1)*(wi_tile/4) + y_off;



This sucks a bit, doesn't it? :)


What do you refer by "the upper point of the tile" ? Oh, I see, you mean the middle point on the tile upper part

Markohs

#208
I guess I'll get some solution to this some day, but can't find it yet.

So I'll post the patch here and a background tile so if anybody wants to test himself, just do it, maybe you catch the bug.

The bug must be in karte_t::draw_background , dunno where.

The background tile is 128x128, it's suited for pak128 but I guess it can work on any pak128 variation too.

https://dl.dropboxusercontent.com/u/30024783/background-8.patch
and
https://dl.dropboxusercontent.com/u/30024783/ground.Outside.pak

and the executable for the curious.

I know the code is far from optimized btw, I just want it to do the job correctly now.

I also know having the image in ground.outside.pak offset is crappy idea maybe too, and that moving the screen too much corrupts the clipping, it goes away when simple drawing is disabled.

I just want to know why the background doesn't align perfectly to the world grid, it just fails in one zoom level, on rest it looks ok.


Dwachs

#209
Quote from: Markohs on April 19, 2013, 03:43:29 PM
I just want to know why the background doesn't align perfectly to the world grid, it just fails in one zoom level, on rest it looks ok.
It looks like accumulated round-off error due to the image size is 128 * 4/3 is not an integer.

Edit:
Using this in display_background reduces the error:

   const int he_bg = (int) get_tile_raster_width() & (~3);

The reason for this is: in simview.cc, the y-screen coordinate is increased by raster_width / 4, so the increment for the background should be four times this value, which is not equal to raster_width but 4*(raster_width/4), the expression above.
Parsley, sage, rosemary, and maggikraut.