Author Topic: [Project] GUI Theme  (Read 98193 times)

0 Members and 1 Guest are viewing this topic.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: [Project] GUI Theme
« Reply #70 on: June 12, 2013, 05:13:42 PM »
My alignment enum has nothing to do with text.

Exactly, and therefore I don't see the need, but that might be because I'm unfamiliar with this way of laying out a GUI.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #71 on: June 12, 2013, 09:17:17 PM »
So what is the conclusion?

Rename the current ALIGN_XXX to TEXT_ALIGN_XXXX so we later can add more text related stuff and keep them separated from object alignment?
- My code doesn't have bugs. It develops random features...

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #72 on: June 12, 2013, 10:34:46 PM »
I wrote I am not very comfortable with using other objects to align. The Alignment needs to be always left vertically center (with the execptions of multiline text), or things will look weird in several languages. It means to give labels a bounding box, yes. Otherwise I fail to see how a left margin can be consistently achieved.

But I have to further look at the design, I am still not fully recovered yet. But most important, three people discussing this there is no conclusion so far. Maybe the conclusion that, whatever will happens, labels need a size.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #73 on: June 12, 2013, 10:40:58 PM »
I'm not talking about aligning text to controls or controls to text.
I guess I just do what I fit is needed and then you will probably see what I mean...

Get well Prissi...
- My code doesn't have bugs. It develops random features...

Offline Max-Max

Re: Make world limits not forced to water level
« Reply #74 on: June 13, 2013, 07:14:10 PM »
While developing this, I noticed there are not much image creation and management routines in the game, just like the climate code does, this patch needs to create new images each time the world is created. kierongreen aproach to this was duplicating the image and rotating it if necessary. It's all implemented around bild_besch_t::copy_rotate.

 I need to create *new* images, with arbitrary size and recode them using them the RLE, do you think it's a good idea to place this routines in bild_besch_t , routines that accept a uncompressed buffer and compress it or it's not the right place to code this?

 Just asking, I'm doing it that way, but wanted to hear your oppinions for the case you point me to a better solution. We could maybe implement that in simgraph16.cc (rezoom routines are there), or in a new collection of routines called bild_tools::* or image_tools:: . Dunno.

I will also need to create images on the fly for the themes later on.

Since we already have functions to draw lines, boxes and text, one approach might be to create a canvas (image context). When calling any draw function we simply provide the canvas we want it to draw on. As default, if no canvas is sent, it uses the current screen canvas (as of today).

A canvas can be a simple structure with.

pointer to data (graphics) or other already defined way of managing images.
current forground color
current background color

This is a commonly used way to do it in most graphic systems.

I do remember seeing somewhere in the code that a 1pixel image was created on the fly if a requested image wasn't defined. But I don't remember the context where it was used :P
- My code doesn't have bugs. It develops random features...

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: Re: Make world limits not forced to water level
« Reply #75 on: June 13, 2013, 07:45:21 PM »
I will also need to create images on the fly for the themes later on.

Since we already have functions to draw lines, boxes and text, one approach might be to create a canvas (image context). When calling any draw function we simply provide the canvas we want it to draw on. As default, if no canvas is sent, it uses the current screen canvas (as of today).

A canvas can be a simple structure with.

pointer to data (graphics) or other already defined way of managing images.
current forground color
current background color

This is a commonly used way to do it in most graphic systems.

I do remember seeing somewhere in the code that a 1pixel image was created on the fly if a requested image wasn't defined. But I don't remember the context where it was used :P


This is a very different graphics system from what Simutrans has now, and what Markohs is working with. Apart from the frame buffer itself, Simutrans revolves around images that are written once, and drawn millions of times (not 100 % true, but descriptive enough). Canvases for GUI should be discussed in a GUI topic.

Offline Max-Max

Re: Re: Make world limits not forced to water level
« Reply #76 on: June 13, 2013, 08:05:58 PM »
Quote
Simutrans revolves around images that are written once, and drawn millions of times
Yes, and this is exactly what I mean. But why invent the wheel again when there already are draw functions available?
However, I don't know how the underlying graphics stuff works, this was only a thought...
- My code doesn't have bugs. It develops random features...

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: Re: Make world limits not forced to water level
« Reply #77 on: June 13, 2013, 08:23:26 PM »
There is nothing in the GUI that I am aware of that isn't loaded from disk and that doesn't scale with the window or change almost every time it is drawn. Avoiding to redraw parts that hasn't changed is or can be handled with mostly with the dirty rectangles on the frame buffer level, as moving windows shouldn't make much impact on overall performance. Images in Simutrans can't change size, have multiple storages for player coloring and zooming, and merge identical images into one, all of which is overkill for GUI stuff. The capacity is also limited, and once allocated, an image can never be freed.

A canvas concept for the GUI might make sense, as it could encapsulate some of the clipping and maybe dirty rectange updating, but trying to build it on to of the image storage sounds like unnecessary pain. It's that pain I'm warning against.

Offline Max-Max

Re: Re: Make world limits not forced to water level
« Reply #78 on: June 13, 2013, 08:47:45 PM »
This is probably the wrong thread to talk about this, but my thought was to create any missing image in the theme (by creating and drawing a standard image for the missing theme image). This would reduce the need to check if an image exist, because there will always exist one.
- My code doesn't have bugs. It develops random features...

Offline Markohs

  • DevTeam, Coder/patcher
  • Devotee
  • *
  • Posts: 1560
  • Total likes: 1
  • Helpful: 86
  • Languages: EN,ES,CAT
Re: Re: Make world limits not forced to water level
« Reply #79 on: June 14, 2013, 12:13:51 AM »
While it's true simutrans lacks code to do what you suggest, Max-Max, current routines are designed to be drawn on the framebuffer. As ters said, images are designed to be loaded just once and written often to framebuffer (scaled or not).

Also, the current code doesn't treat them in a dynamic-friendly way, simutrans basically loads all the pak images on memory (compressed) and assigns them a number. The only dynamic images implemented are the terrain ones and they are done in a somehow hackish way (imho). They are just created and the id of the first registered one is stored, if the map is re-created all images with an id greater than that one are deleted.

 What I'm trying to express is that simutrans images and routines are designed to be basically static. Is that a bad thing? No, it's the way this game was designed and the code is certainly robust and performs well enough.

 Do we need to modify this? I'm not sure, probably yes, when there is a need to (if they are really useful somewere and make a difference). Personally I think it would be a good idea implementing some new routines in the style you mentioned, but I'd do it as a complementary system, letting current routines unchanged, because they need to be lightning fast. But I kind of think your idea is not bad.

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4069
  • Total likes: 98
  • Helpful: 146
  • Languages: EN, DE, AT
Re: Re: Make world limits not forced to water level
« Reply #80 on: June 14, 2013, 06:03:04 AM »
The only dynamic images implemented are the terrain ones and they are done in a somehow hackish way (imho). They are just created and the id of the first registered one is stored, if the map is re-created all images with an id greater than that one are deleted.
... which is no longer true. These images are generated once on startup.

Max-Max, I do not know whether I understand your intent: Do you need to create dynamic images and want to use them? Ie use existing draw routines to modify your image buffer.
Parsley, sage, rosemary, and maggikraut.

Offline Markohs

  • DevTeam, Coder/patcher
  • Devotee
  • *
  • Posts: 1560
  • Total likes: 1
  • Helpful: 86
  • Languages: EN,ES,CAT
Re: Re: Make world limits not forced to water level
« Reply #81 on: June 14, 2013, 08:51:48 AM »
... which is no longer true. These images are generated once on startup.

 oooh I stand corrected, sorry. :)

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: Re: Make world limits not forced to water level
« Reply #82 on: June 15, 2013, 10:21:49 PM »
This gets slowly offtopic, but: In principle you can have easily a canvas by pointing textur to something else while drawing the dialogue. It will fail however with the allegro backend, as it uses a pointer to a screen area.

Nevertheless, most dialogues are dynamic (Speed of train, poduction of town, money, statistics, incomes, World view, ...) The only static ones are the option dialogue. Thus I am not sure the double buffering is useful. It would need a class of buffered and non-buffered dialogues. And I fear the class of the latter is very small compared to the maximum gain from it.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #83 on: June 19, 2013, 09:48:33 AM »
I have managed to isolate the basics into one patch.

This patch will not make any visual changes (what I'm aware of), but implements the D_TITLE_HEIGHT in simwin and werkzeug_waehler.
I will try to make more patches, one for each dialogue.
- My code doesn't have bugs. It develops random features...

Offline Max-Max

Re: Re: Make world limits not forced to water level
« Reply #84 on: June 19, 2013, 03:27:06 PM »
Hmm, I think we have different definitions of what an static/dynamic image is. I will try to illustrate what I mean. Consider this piece of code:

arrow_left_normal = skinverwaltung_t::window_skin->get_bild_nr( 8 );

If  image 8 isn't loaded (it is missing in the PAK file) I want to allocate a new blank image in its place. On this blank image I wan to draw a simple left arrow. Now after the creation of this missing image...

arrow_left_normal = skinverwaltung_t::window_skin->get_bild_nr( 8 );

...will use the image of my simple arrow. So the image is static in the sense that its image data is not changed, but created dynamic in memory if the skin image is missing in the PAK file.

I'm not talking about double or triple buffering used to minimize flicker. I think Dwachs is closest to my intention; Yes it would be nice if it was possible to use the current drawing routines like display_fillbox etc... to create this missing image.

The image 8 is only to illustrate an example, it could practically be any image loaded from a PAK file at load.
« Last Edit: June 19, 2013, 10:04:56 PM by Max-Max »
- My code doesn't have bugs. It develops random features...

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: Re: Make world limits not forced to water level
« Reply #85 on: June 19, 2013, 06:09:22 PM »
I don't think the game should make images that are missing. It's easier to just draw simple placeholders than to code something that draws them anyway. In some cases, the size of the image is information the game needs, so it needs to have the image in the first place just to know what size it should be.

Offline Max-Max

Re: Re: Make world limits not forced to water level
« Reply #86 on: June 19, 2013, 08:29:52 PM »
Okay, now this has really become of-topic...

But the game is already doing it now (have a look in gui_button.cc). The sizes and images are already hard coded for missing skin images and redrawn using display_xxxx every frame, instead of simply replacing the missing image with the one it has already drawn.

By replacing the missing images (that you draw every frame anyway), you only have one scenario and less code to maintain. How on earth can that be a bad thing?!?
- My code doesn't have bugs. It develops random features...

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: Re: Make world limits not forced to water level
« Reply #87 on: June 19, 2013, 08:38:26 PM »
I'm just saying it's a lot of work for little gain. I was also assuming that the game didn't fake missing images in any way whatsoever. Either they exist in some file, or the game won't start. (I have never really read GUI code, just seen some in passing. My studies have been in the besches, landscape data structures, and low level system and rendering code.)

Offline Max-Max

Re: Re: Make world limits not forced to water level
« Reply #88 on: June 19, 2013, 09:20:13 PM »
...I have moved this to the GUI theme thread.
« Last Edit: June 19, 2013, 09:26:43 PM by Max-Max »
- My code doesn't have bugs. It develops random features...

Offline Max-Max

Re: [Project] GUI Theme
« Reply #89 on: June 19, 2013, 09:26:05 PM »
This is a continued discussion.

Before we got completely of-topic, Markohs said...
Quote
I need to create *new* images, with arbitrary size and recode them using them the RLE, do you think it's a good idea to place this routines in bild_besch_t , routines that accept a uncompressed buffer and compress it or it's not the right place to code this?
...and I said I need it too. I only suggested to use a canvas/context device/what-ever so we could reuse the functions that are already in place.
This isn't really my area, but I had a quick look in simgraph16.cc and found that a few routines display_xx_internal are writing to a pre-calculated memory pointer...

PIXVAL *p = textur + xp + yp * disp_width;

Isn't it possible to add a parameter to these routines that is NULL by default. If NULL use the pre-calculated pointer, PIXVAL *p = textur + xp + yp * disp_width; other wise, use whatever type this extra parameter needs to be, a bild_t* ?
« Last Edit: June 21, 2013, 02:06:18 PM by Fabio »
- My code doesn't have bugs. It develops random features...

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #90 on: June 19, 2013, 09:40:59 PM »
There are routines in the simgraph16.cc, which does the trick (rezoom_img does unpack and repack images).

But again: What is the purpose? Buffering dialogues in simutrans does no make sense, since most dialogues can change very much between each update.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #91 on: June 19, 2013, 10:03:19 PM »
No, this has nothing to do with buffering dialogues or render buffers. Read this post for an example.

Today, the code is already drawing missing skin elements every frame with the display_xxx functions. I want to create the missing skin images after load so we can use the same code everywhere and not draw them every time, like we do in gui_button.cc for each missing skin element.

By creating the missing skin images after load, we can remove the fall back code in the GUI, because there will always be a skin image available...
- My code doesn't have bugs. It develops random features...

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #92 on: June 19, 2013, 11:17:13 PM »
If you want images, just require all images or simutrans will not load?

Those are dated back from times when the central skin directory did not exist. Nowadays it is very easy to make sure using this that a skin with images exist.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: [Project] GUI Theme
« Reply #93 on: June 20, 2013, 04:19:45 AM »
Maybe we should just trim the fallbacks from trunk? One less thing to maintain.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #94 on: June 20, 2013, 08:34:23 AM »
Some pak still need them, as some symbols are taken from symbol.misc.pak Thus, as soon as the new system is ready, then this code can go at the same time.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #95 on: June 21, 2013, 02:44:57 AM »
I have put up the two patches in the first post. These patches would make any difference, but they prepare the code with the basics to create one patch per dialogue.

By setting the THEME_TEST (in gui_frame_t.cc) to 1 you can switch between a test setting and "normal" settings.
At this point the test setting is of course not doing much more than screwing up the dialogues, but I will fix them one by one...
- My code doesn't have bugs. It develops random features...

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #96 on: June 21, 2013, 08:48:59 AM »
Just attach them here next time; it save some scrolling. Otherwise also the discussion will be very difficult to follow as soon as new versions are released.

Short comments:
First, you are using spaces for indent. Please all indent until the first character are tabs, if possible. Only for ifs and for alignment of defs spaces are ok.

draw_roundbutton: You left the "if(h==14)" in there, despite the button now enforcing BUTTON_HEIGHT. Should this be either an magic enum or rather BUTTON_HEIGHT?

I am not happy about TEXT_ALIGN_xxx and ALIGN_xxx As discussed previously, those do the same, thus they should use the same name exactly. (It also blows out the patch size considerably.)

You still follow the idea of align stuff right or left to elements. In which situation this is needed? With very few exceptions, the only align used would be on a vertical position, since the horizontal line must be fixes or else the dialogues would be very randomly designed, depending on language an all. If you really want to do this, then all labels need an explicit groesse (which then also automatically would take care of the vertical alignment). Otherwise larger font or different language (and thus text) will mess up the alignment thouroughly. (Especially since it will affect groesse.x!) BTW: One of the few places, where left/right alignment make actually sence (in numberinput) you did not used it.

Also the offset is something different to alignment. Not too happy to combine everything there, but well.

What purpose should ALIGN_EXTERIOR_V serve? That could be easily passed as an offset. (Make make an offset only routine too?)

The D_GET_CENTER_ALIGN_OFFSET/D_GET_FAR_ALIGN_OFFSET should not be needed with the align routines and properly enforced bounding boxes for labels. Furthermore, they just high a subtraction (and a shift), thus are longer than their explicit writing down.

Shouldn't the BUTTON_X() take into account the left border of a dialog, i.e. D_MARGIN_LEFT?

Switching themes should be done by loading stuff. But ok, for testing rather include something into the gui_frame_i init.

SKIN_CELL_SIZE is imho not needed.

ALso why did you moved the button typ definition in welt.cc? Does you routine depend now on it? The changing button types must be avoided at all.

Sorry, I have to work now, but thanks for this. It is a good way to start.

Offline Max-Max

Re: [Project] GUI Theme
« Reply #97 on: June 21, 2013, 01:42:04 PM »
I did attached the first one of them in the same reply once, but it only got one download and no comments at all, so I figured no one saw it. But sure, I can attach them to the same reply from now on.

Quote
First, you are using spaces for indent. Please all indent until the first character are tabs, if possible. Only for ifs and for alignment of defs spaces are ok.
When I asked before I got the answer first indent as tab character with size 4 spaces, then spaces after that with size of 2. How about updating the document so there can't be any misunderstandings. Are you referring to block indents? I think the best would be if you just show an example.

Quote
draw_roundbutton: You left the "if(h==14)" in there, despite the button now enforcing BUTTON_HEIGHT. Should this be either an magic enum or rather BUTTON_HEIGHT?
If you read the comment after it "// Max: Replace 14 with the actual button image height". The routine assumes that the image for a button always is 14 pixels in height, so even if there is an image present but the button height isn't 14 pixels, it is drawn with the fall back routine. I plan a different solution in drawing the theme elements and don't want to do too many changes at this point.

Quote
I am not happy about TEXT_ALIGN_xxx and ALIGN_xxx As discussed previously...
...You still follow the idea of align stuff right or left to elements...
...Also the offset is something different to alignment...
...What purpose should ALIGN_EXTERIOR_V...
Maybe I should write a document with pictures to illustrate how it works. Again this has nothing to do with text at all. I have a plan with this alignment stuff and further down the road it will be used more frequently. Remember, one of the goals are targeting portable devices where the screen size is very limited. As I described earlier, I have a plan for how to handle this.
I'm not sure if anyone understands what exactly is aligned against what. I can replace the TEXT_ALIGN_XXX with the ALIGN_XXX if some one can solve the two other flags in that enum, DT_DIRTY and DT_CLIP (get them out of there). As you said yourself, they don't belong in there...

Quote
The D_GET_CENTER_ALIGN_OFFSET/D_GET_FAR_ALIGN_OFFSET should not be needed...
You are absolutely right here, this was actually what lead to the alignment routine in the first place, because I saw that the alignment need was quite big. It will be replaced and removed.

Quote
Shouldn't the BUTTON_X() take into account the left border of a dialog, i.e. D_MARGIN_LEFT?
It makes sense not not include the margin. If you are placing them in relation to something else than the border, you don't want the margin to be included. Secondly, the child object should not be depending on its parent's layout. I'm planing the concept of client areas further down the road.

Quote
Switching themes should be done by loading stuff.
I thought we agreed on doing this in very small steps... I haven't even started on the theme loading yet. Still only replacing magic numbers.

Quote
SKIN_CELL_SIZE is imho not needed.
I wasn't sure what this really meant. Does it mean that a skin image can't be smaller or larger than 64 pixels? I put it in there for further elaboration.

Quote
ALso why did you moved the button typ definition in welt.cc?
The set type function gives a default size to the control. I use to replace them with init() when I can. Unfortunately, for some reason, Init() isn't a member in the base class gui_komponente_t and can't be used for all components. I'm leaning towards to implement the Init in the base class. But as you said, I don't want to mess around to much with he classes now, I focus on the magic numbers.
In this special case it has been replaced with the more appropriate Init() function, but you wanted all dialogues, one by one, so I had to do something to fix the issue temporarily in this patch.

I was told to not release large patches and to release one dialogue at the time. To do this I can't implement all at once and as I said before, the same code may change many times on the road. So either you will have many small patches, some obsoleting old ones, or you can wait and get one stage at a time in a huge patch. Your call...
- My code doesn't have bugs. It develops random features...

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #98 on: June 21, 2013, 09:38:29 PM »
Space instead tabs are on many places. Just open your file with notepad, and you will immeadiatly see ifs, that have tabs and three lines below spaces. For instance the case in line 187.

The DIRTY/CLIP flags with text could still be part of the alignment enums. Just use a higher number, 0x40 and 0x80 are not used.

Buttons were the oldest element; that is the reason they behave different and have an init. Also the world creation dialogue was (is) able to change its language on the fly; this is no longer needed. Hence a type changing the size of the element is really a nono. This is an unexpected side effects and should be really avoided. (As also said for the labels: Their size MUST NOT depend on the text it displays, but must be given on initialisation.) The only way for proper bounding boxes is, that those are set by the programmer (using the BUTTON_WIDTH and the like).

An init routine in gui_komponente does not make much sense or? It has to have different parameter for each component. One could however argue for an init routine to be present for any implemented gui_komponent_t.

The skin (i.e. dialogue background) should be able to cope with any size (and even allow for now skin with transparency, like the chat window.) Its being 64 is rather tradition and not really needed.

Why does the gui_frame_t needs to know the size of a gadget? Should this (and the titlebar) rather go to simwin.cc ultimatively?

Quote
It makes sense not not include the margin. If you are placing them in relation to something else than the border, you don't want the margin to be included. Secondly, the child object should not be depending on its parent's layout. I'm planing the concept of client areas further down the road.
Please elaborate. It seems you target are dialogues, which change their layout considerably when changing languages or switching sizes (instead just scale up). Or is this a misunderstanding?

But, all in all this patch is progressing in such a way that it can be tested and then integrated.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: [Project] GUI Theme
« Reply #99 on: June 21, 2013, 10:20:44 PM »
When I asked before I got the answer first indent as tab character with size 4 spaces, then spaces after that with size of 2. How about updating the document so there can't be any misunderstandings. Are you referring to

Who wrote this? Having the first indent level twice as large as the rest don't make any sense at all. It's also a bit contradictory that it says use a tab character of four spaces. A coding style either uses tab character, or it uses spaces. Maybe it's meant to say that the editor should be set up with a tabulation size equal to the length of four spaces, but possibly the best reason for using tab in the first place is so that each can adjust indent width to their own preference.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 8569
  • Total likes: 254
  • Helpful: 226
  • Languages: De,EN,JP
Re: [Project] GUI Theme
« Reply #100 on: June 22, 2013, 08:45:35 PM »
After some more thinking:

Many simutrans dialogs have row of objects. Meaningful alignmet therefore would be a function like:
align( GUI_komopnenete_t *h_pos, gui_komponente_t *v_pos, alignenum a, koord offset );

What do you think?

Offline Max-Max

Re: [Project] GUI Theme
« Reply #101 on: June 24, 2013, 03:41:04 AM »
Despite the midsummer celebration here in Sweden, I have put together some of my thoughts around the new Theme system in this draft.

Remember this is only thoughts and ideas, not the law :police:

Quote
Space instead tabs are on many places. Just open your file with notepad, and you will immeadiatly see ifs, that have tabs and three lines below spaces. For instance the case in line 187.
Line 187 in what file? Wouldn't be easier to just update the code document instead of pointing out all the errors, show how it should be done, not where it is wrong.

Quote
The DIRTY/CLIP flags with text could still be part of the alignment enums. Just use a higher number, 0x40 and 0x80 are not used
.
But they have nothing to do with alignment. Especially not GUI bounding boxes against other GUI bounding boxes. Imho I think these flags has less to do with my alignment, than my alignment has to do with text alignment. My routines don't use them at all. I still think that text alignment should be in its own enum, together with these flags.
There is 2 bits left and I do have a plan to use these two bits further down the road. That is why they are reserved.

Quote
Buttons were the oldest element; that is the reason they behave different and have an init. Also the world creation dialogue was (is) able to change its language on the fly; this is no longer needed. Hence a type changing the size of the element is really a nono. This is an unexpected side effects and should be really avoided. (As also said for the labels: Their size MUST NOT depend on the text it displays, but must be given on initialisation.) The only way for proper bounding boxes is, that those are set by the programmer (using the BUTTON_WIDTH and the like).
Almost all button init() sets the size to koord( D_BUTTON_WIDTH, D_BUTTON_WIDTH ). I made it possible to exclude the size and have the Init() to set them to the default size for that control. You can still override this in the init() by setting the size parameter. If this isn't still enough you can set a new size after the init() call.

Quote
An init routine in gui_komponente does not make much sense or? It has to have different parameter for each component. One could however argue for an init routine to be present for any implemented gui_komponent_t.
It makes all the sense in the world. All components has a position and a size, this can be set through the init() routine.
As you know, in C++ you can have more than one version of a function as long the number of parameters and types isn't the same as declared earlier. If you need more params to init a control, just make a new Init routine, call the old one and then handle the new parameters.
If you want to forbid calls to the older version of Init() you can move it to be protected or even private in the descendent class.

Quote
The skin (i.e. dialogue background) should be able to cope with any size (and even allow for now skin with transparency, like the chat window.) Its being 64 is rather tradition and not really needed.
Do we have support for transparent images now? The makeobj is complaining if it finds an alpha layer.

Quote
Why does the gui_frame_t needs to know the size of a gadget? Should this (and the titlebar) rather go to simwin.cc ultimatively?
I think I mentioned it earlier. I put some extra stuff in there so it would be easy to test, not having to open several files to tweak and test. This stuff will be moved later on...

Quote
Please elaborate. It seems you target are dialogues, which change their layout considerably when changing languages or switching sizes (instead just scale up). Or is this a misunderstanding?
Hmm, maybe. My alignment and window resize will not change the size of the GUI components other than outlined in the document I wrote. The collapse functions will be one of the last stages in this project.

Quote
Many simutrans dialogs have row of objects. Meaningful alignmet therefore would be a function like:
align( GUI_komopnenete_t *h_pos, gui_komponente_t *v_pos, alignenum a, koord offset );
To organise controls in rows, columns and grids will be handled by a layout control. The alignt_to() is the basic needs for the layout controls to work with. But who knows, maybe it changes down the road...

And for you Ters
Quote
Who wrote this?
Does it really matter? How would that information move this project forward?

Quote
Having the first indent level twice as large as the rest don't make any sense at all.
It might not make any sense to you, but it does to me and many others...

Quote
It's also a bit contradictory that it says use a tab character of four spaces. A coding style either uses tab character, or it uses spaces
In fact several IDE use tab characters as far as possible and then mix in with space to reach the intended indent.


Regarding this tab & space hickup...
If you guys had spent a few minutes to update the code style document, as I have requested several times, we wouldn't even have this discussion now...
I think you have spent more time, words and efforts to tell me how stupid my code is than it would have taken you to update the document.
« Last Edit: June 24, 2013, 03:50:19 AM by Max-Max »
- My code doesn't have bugs. It develops random features...

Offline neroden

  • Devotees (Inactive)
  • *
  • Posts: 831
  • Total likes: 0
  • Helpful: 28
  • Nathanael Nerode
Re: [Project] GUI Theme
« Reply #102 on: June 24, 2013, 03:57:34 AM »
MaxMax: I think this project is great.  I have one strong worded piece of advice.

While you're rewriting the gui, do *not* use the koord class.  It is used (inconsistently) in parts of the existing gui, and it would be good if it were not used.  Koord should be for map coordinates.  Only.

If you need a similar class for (x, y) points in your gui, please make a new class and we can use that one for screen coordinates.  Rewriting the gui to stop interacting with the very complicated koord class would allow for:
(1) better debugging (gdb spews a lot of noise when dumping 'koord' due to all the static elements)
(2) better type-safety
(3) larger screens.  Koord is limited to roughly 2^15 by 2^15 bits, and we don't want to increase it (right now) because that would affect a lot of gameplay code.  However, there's no reason why the screen should be limited to that number of pixels.

----
On the topic of spaces vs. tabs and so forth: I have noticed that simutrans does not have a standard prettyprinting/indentation style.   It might be worth considering having *some* standards.

I like having loose standards, since some functions need to be indented differently from others.  And given the divergences between standard and experimental, and the number of outstanding patchsets, now is probably not a good time to establish a strict indentation style.

Does everyone have access to GNU indent or a program with similar options?  What is the equivalent on Windows?
Personally I like "Linux kernel style" (indent --linux-style) pretty well:
Code: [Select]
int x;
if (x) {
} else {
}
I don't like GNU style (wastes too many lines, too much whitespace):
Code: [Select]
int  x;
if (x)
{
}
else
{
}

I guess simutrans does have one rule: tabs for indentation, one tab per indent, no spaces.  I like this.  This would be equivalent to something like "indent  -ts4 -i4 -ci4 -cli4 -bli0 -cbi0"  (where all the numbers are multiples of the ts number).
---
Reading through the outdated coding styles document, I see that my preferred bracket style is preferred :-)  I really really strongly dislike the official simutrans preferred placement of pointer marks and I would suggest that the style recommendation be reversed.  char* x; is a good declaration, char *x; is a hard-to-read muddle.  I would however prohibit declaring multiple variables on the same line in order to avoid the problems related to that.
« Last Edit: June 24, 2013, 04:18:33 AM by neroden »

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 4453
  • Total likes: 141
  • Helpful: 105
  • Languages: EN, NO
Re: [Project] GUI Theme
« Reply #103 on: June 24, 2013, 04:52:06 AM »
Wouldn't be easier to just update the code document instead of pointing out all the errors, show how it should be done, not where it is wrong.
If it was easy, it would have been done by now. Reminds me of some guidelines on Wikipedia that "everybody" knows are not as they should, but for which they can't reach consensus on a new text, so the old text stays and they just disregard it.
Does it really matter? How would that information move this project forward?
Not this project, but it might help us put a stop to misinformation in the future.
It might not make any sense to you, but it does to me and many others...
In fact several IDE use tab characters as far as possible and then mix in with space to reach the intended indent.
I have never seen coding styles where indentation sizes are not uniform. As for mixing tabs and spaces, it's the worst possible way to indent, unless there is a clear separation between when to use tabs (to indent code) and spaces (to indent comments after code). I have seen it used so that the first indent level is say four spaces, the second is one tab, the third is tab plus four spaces, then two tabs and so on. Unlike the alternatives, it won't look right in other editors with different tabulation sizes. It can also interfere with search and replace operations including the indentation. An abomination in my opinion.
 
I agree with neroden that koord is for the map, not the GUI.

Offline kierongreen

Re: [Project] GUI Theme
« Reply #104 on: June 24, 2013, 07:44:45 AM »
Preferred coding style is
if(  x && y  ) {
}
else {
}