DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Advice on this code

Sat Sep 12, 2020 2:15 am

Hello everyone,

I need some advice on this code it's only a proof of concept and not complete. It will compile as I added some test code to it. I feel like I might be going about this the wrong way.

Code: Select all

#include<iostream>

typedef enum {
    UIE_Button,
    UIE_Toggle,
    UIE_Lamp,
    UIE_Switch,
    UIE_Alert,
    UIE_ProgressBar,
    UIE_Close
} UIE_TYPES;

class Element
{
    void (*Click)();
    public:
    UIE_TYPES       e_type;
    const char*     caption;
    int             pos_x;
    int             pos_y;
    int             pos_w;
    int             pos_h;
    int             colour;
    int             state;
    int             value;

    Element(void (*CB)(),UIE_TYPES UI_type, int x, int y, int w, int h, int c, const char* cap)
        : Click(CB), e_type(UI_type), caption(cap), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(0), value(0)
    {}
    Element(void (*CB)(),UIE_TYPES UI_type, int x, int y, int w, int h, int s, int c, const char* cap)
        : Click(CB), e_type(UI_type), caption(cap), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(s), value(0)
    {}
    Element(UIE_TYPES UI_type, int x, int y, int w, int h, int c, int v)
        : Click(NULL), e_type(UI_type), caption(0), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(0), value(v)
    {}

    bool HitBox(int x, int y)
    {
        if (x > pos_x and x < pos_x + pos_w)
            if (y > pos_y and y < pos_y + pos_h)
            {
                if (Click) Click();
                return true;
            }
        return false;
    }

    void Render()
    {
        switch (e_type)
        {
            case UIE_Button :
                //draw ();
                break;
            case UIE_Toggle :
                //draw ();
                break;
            case UIE_Lamp :
                //draw ();
                break;
            case UIE_Switch :
                //draw ();
                break;
            case UIE_Alert :
                //draw ();
                break;
            case UIE_ProgressBar :
                //draw ();
                break;
            case UIE_Close :
                //draw ();
                break;
        }
    }
};

class Window
{
    // PRIVATE DATA
    unsigned char   m_id;
    const char*     m_name;
    Element*        m_children[16];
    int             m_index;
    public:
    Window(unsigned char id, const char* name) // constructor
        : m_id(id), m_name(name)
    {

    }

    void Add_Element (Element* child)
    {
        m_children[m_index] = child;
        m_index++;
    }

    const char* Get_Title()
    {
        return m_name;
    }

    int Get_Count ()
    {
        return m_index;
    }

    int Click(int x, int y)
    {

        for (int i = 0; i < m_index; i++)
        {
            Element E = *m_children[i];
            if (E.HitBox(x,y)) return i;
        }
        return -1;
    }
};

void btn1_CB()
{
    // Do something useful 
    std::cout << "Button 1 CLICKED\n";
}

int main ()
{
    Window page0(0, "Main");  // declare root window
    Element btn1(&btn1_CB, UIE_Button, 10, 10, 100, 100, 1, "Button 1"); // declare new button
    page0.Add_Element(&btn1); // add button to window

    std::cout << page0.Get_Title() << " : " << page0.Get_Count() << "\n";

    std::cout << "CLICK 102, 5\n";
    std::cout << page0.Get_Title() << " : " << page0.Click(102,5) << "\n";
    
    std::cout << "CLICK 15, 25\n";
    std::cout << page0.Get_Title() << " : " << page0.Click(15,25) << "\n";
    
    return 0;
}
The idea was to create a window class that contains my elements and the active window will be fed the input events and flip through it's children to and see if they are clicked. I'm using Call Back functions for the click events this isn't what I was thinking in the beginning but I'm not sure how else to do it. I'm still learning C++ and I know this works however is it a good way to do things? I think there is a better way.

Thanks for looking.

In case a project background will be helpful,
I'm writing a simple HMI based off of my own framebuffer and input libraries this is only meant to run on my Pi 3B+ with the official touch screen, no mouse or keyboard. The OS is a very simple bare bones BuildRoot image.

deepo
Posts: 605
Joined: Sun Dec 30, 2018 8:36 pm
Location: Denmark

Re: Advice on this code

Sun Sep 13, 2020 7:18 pm

I just browsed your code and found this:
Your Window::Click() function copies the elements before calling the HitBox() function.
That takes time. I would just use the pointers:

Change this:

Code: Select all

int Click(int x, int y)
{
    for (int i = 0; i < m_index; i++)
    {
        Element E = *m_children[i];
        if (E.HitBox(x,y)) return i;
    }
    return -1;
}
Into this:

Code: Select all

int Click(int x, int y)
{
    for (int i = 0; i < m_index; i++)
    {
        Element *E = m_children[i];
        if (E->HitBox(x,y)) return i;
    }
    return -1;
}
But maybe the compiler will fix that itself that during optimization, I don't know.

/Mogens

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Sun Sep 13, 2020 8:55 pm

Thanks for having a look,

That make sense I was thinking I was looking at original because I was using a pointer but I see I wasn't, thanks.

I'm thinking to use sub classes of my Element Class for each type of UI Control and then use virtual functions for Drawing and incoming events. I've been learning bits and pieces of what I need/want to do. I just learned about virtual function and how I can use them in this context.

If I have it right this is my Element Class parent class

Code: Select all

typedef enum {
    UIE_MOVE,
    UIE_CLICK
} UIE_EVENTS;

class Element
{
    void (*Click)();
    public:
    UIE_TYPES       e_type;
    const char*     caption;
    int             pos_x;
    int             pos_y;
    int             pos_w;
    int             pos_h;
    int             colour;
    int             state;
    int             value;

    Element(void (*CB)(),UIE_TYPES UI_type, int x, int y, int w, int h, int c, const char* cap)
        : Click(CB), e_type(UI_type), caption(cap), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(0), value(0)
    {}
    Element(void (*CB)(),UIE_TYPES UI_type, int x, int y, int w, int h, int s, int c, const char* cap)
        : Click(CB), e_type(UI_type), caption(cap), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(s), value(0)
    {}
    Element(UIE_TYPES UI_type, int x, int y, int w, int h, int c, int v)
        : Click(NULL), e_type(UI_type), caption(0), pos_x(x), pos_y(y), pos_w(w), pos_h(h), colour(c), state(0), value(v)
    {}

    bool HitBox(int x, int y)
    {
        if (x > pos_x and x < pos_x + pos_w)
            if (y > pos_y and y < pos_y + pos_h)
            {
                if (Click) Click();
                return true;
            }
        return false;
    }

    virtual void Draw(bool force)
    {

    }

    virtual void Event(UIE_EVENTS e, int x, int y)
    {

    }
};
Then I would change the HitBox code (HitBox is an awful name) to something more like this.

Code: Select all

 bool HitBox(UIE_EVENTS e, int x, int y)
    {
        if (x > pos_x and x < pos_x + pos_w)
            if (y > pos_y and y < pos_y + pos_h)
            {
                this.Event(e,x,y);
                return true;
            }
        return false;
    }
Is that correct? I'll have to try it out. But the idea is I can handle mouse moving over and clicking as well as my touch click that I'm after. One thing in the tutorial that I thought sounded incorrect was that a sub class doesn't inherit the private member variables? I'm thinking this is just how it was presented and all the private members exist in the subclass.

User avatar
Paeryn
Posts: 3046
Joined: Wed Nov 23, 2011 1:10 am
Location: Sheffield, England

Re: Advice on this code

Sun Sep 13, 2020 11:59 pm

DarkElvenAngel wrote:
Sun Sep 13, 2020 8:55 pm
One thing in the tutorial that I thought sounded incorrect was that a sub class doesn't inherit the private member variables? I'm thinking this is just how it was presented and all the private members exist in the subclass.
A derived class inherits all the variables from the base class and most of its methods (*). However anything that the base class declared private is inaccessible to the derived class.

(*) The base class' friend functions, overloaded operators and constructors, destructors & copy constructors are not inherited by derived classes. I think that's all that aren't, not too up on the latest standards.
She who travels light — forgot something.
Please note that my name doesn't start with the @ character so can people please stop writing it as if it does!

LdB
Posts: 1622
Joined: Wed Dec 07, 2016 2:29 pm

Re: Advice on this code

Mon Sep 14, 2020 4:38 am

Code: Select all

bool HitBox(UIE_EVENTS e, int x, int y)
{
     if (x > pos_x && x < pos_x + pos_w)
     if (y > pos_y && y < pos_y + pos_h)
    {
        this.Event(e,x,y);
        return true;
    }
   return false;
}
Depending how your c-oords work I am not convinced it shouldn't be >= and <=

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Mon Sep 14, 2020 3:34 pm

LdB wrote: Depending how your c-oords work I am not convinced it shouldn't be >= and <=
The input events should match exactly with the screen position. The Elements are simple rectangles and have a 1 pixel boarder. Your right if the mouse is used and is over the boarder it will not register and it gives touch a better chance too.

Thanks for that catch!

deepo
Posts: 605
Joined: Sun Dec 30, 2018 8:36 pm
Location: Denmark

Re: Advice on this code

Mon Sep 14, 2020 8:58 pm

Another few notes:

If you have a lot of elements you are looping through to find out whether the mouse clicked on one of them, then you should have way out of that loop, when you have found the element that the mouse clicked.
That way you don't have to run through a lot of elements after having processed the mouse click.

You should also have a Z value of your elements, that reflect what is behind what.
That way you can handle mouse clicks on elements that are on top of each other, only allowing the topmost element to receive the click.

The Z index of the elements need to be updated dynamically when you bring a window in front of another.

/Mogens

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Mon Sep 14, 2020 11:10 pm

Paeryn wrote:
DarkElvenAngel wrote:
Sun Sep 13, 2020 8:55 pm
One thing in the tutorial that I thought sounded incorrect was that a sub class doesn't inherit the private member variables? I'm thinking this is just how it was presented and all the private members exist in the subclass.
A derived class inherits all the variables from the base class and most of its methods (*). However anything that the base class declared private is inaccessible to the derived class.

(*) The base class' friend functions, overloaded operators and constructors, destructors & copy constructors are not inherited by derived classes. I think that's all that aren't, not too up on the latest standards.
I think I get this the sub class cannot access the parent classes copies of the private members only it's own? I didn't think you could access the parent classes members
deepo wrote:
Mon Sep 14, 2020 8:58 pm
Another few notes:

If you have a lot of elements you are looping through to find out whether the mouse clicked on one of them, then you should have way out of that loop, when you have found the element that the mouse clicked.
That way you don't have to run through a lot of elements after having processed the mouse click.

You should also have a Z value of your elements, that reflect what is behind what.
That way you can handle mouse clicks on elements that are on top of each other, only allowing the topmost element to receive the click.

The Z index of the elements need to be updated dynamically when you bring a window in front of another.

/Mogens
I believe I already have the way out as the Click method will cycle through each Element Class and when HitBox returns true I return the element's index. Does this not do what I think it does? I saw something about iterators but I wonder if that could replace the for loop or if it's basically the same thing in this case?

I'm thinking about the Z Indexing and it seems like that's going to be very complex to make work efficiently. The idea of a window in my program is solely to contain the Elements, it's position is fix on the screen the screen can only display one window at a time.

Code: Select all

SCREEN SIZE = 800 x 480
+-----------------------+-------+
|			|	|
|			|	|
|	PAGE		|  SIDE	|
|	AREA		|  BAR	|
|			|	|
|			| <   >	|
+-----------------------+-------+
|	STATUS AREA? 		|
+-------------------------------+
This is my screen layout, the Page Area is where a window will be displayed. The Side Bar will have the Exit Button and window cycle controls So I can change which window is displayed.

There might be an easy way to implement this but I'm not seeing it or it uses something I don't yet know or have a good grasp on.

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Tue Sep 15, 2020 11:23 pm

Okay I put my classes to real work they are doing a good job one thing is driving me nuts though.

Code: Select all

Element btn1(&btn1_CB, UIE_Button, 10, 10, 100, 100, 1, "Button 1");
    page0.Add_Element(&btn1);
    Element btn2(&btn1_CB, UIE_Button, 110, 10, 100, 100, 1, "Button 2");
    page0.Add_Element(&btn2);
    Element btn3(NULL, UIE_Button, 10,110,100,100,2, "Button 3");
    page0.Add_Element(&btn3);
    Element btn4(NULL, UIE_Button, 210,110,100,20,2, "Button 4");
    page0.Add_Element(&btn4);
    Element btn5(NULL, UIE_Button, 210,140,100,20,2, "Button 5");
    page0.Add_Element(&btn5);
    Element btn6(NULL, UIE_Button, 210,170,100,20,2, "Button 6");
    page0.Add_Element(&btn6);
    Element btn7(NULL, UIE_Button, 210,210,100,20,2, "Button 7");
    page0.Add_Element(&btn7);
There must be a way to change the code so I don't have to create each element before I add it. I'm sure that has a special name because I'm looking for it and cannot find an example.

Would it be something I do with a static call or do I overload the add element function? I feel like I'm wasting memory. I could even make a function for each element type.

I feel like the using new to allocate the memory is a way to go then add a destructor with delete at the end. Don't want memory leaks.

Heater
Posts: 16515
Joined: Tue Jul 17, 2012 3:02 pm

Re: Advice on this code

Tue Sep 15, 2020 11:39 pm

I am told one should never write "new" or "delete" in modern C++ code. And avoid using raw pointers as much as possible.
Memory in C++ is a leaky abstraction .

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Wed Sep 16, 2020 12:13 am

Heater wrote: I am told one should never write "new" or "delete" in modern C++ code. And avoid using raw pointers as much as possible.
I see I read up on this a bit more over here https://stackoverflow.com/questions/650 ... use-of-new Interesting stuff. And since I have C code doing memory allocations I shouldn't do it! I see why you say C++ is a headache.

So looking at that if I try to define everything on the stack then what I'm doing is the best way to go the other way would be defining all 16 elements in the window class and then setting them up as I add them, not clean or efficient.

Heater
Posts: 16515
Joined: Tue Jul 17, 2012 3:02 pm

Re: Advice on this code

Wed Sep 16, 2020 12:38 am

DarkElvenAngel wrote:
Wed Sep 16, 2020 12:13 am
So looking at that if I try to define everything on the stack...
That is not what I meant about not using "new", "delete", and raw pointers.

It's more about using modern C++ and so called "smart pointers". "shared_ptr", "unique_ptr" and whatever.

Did I say there is a reason I don't want to use C++ anymore?
Memory in C++ is a leaky abstraction .

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Wed Sep 16, 2020 1:51 am

Heater wrote:
Wed Sep 16, 2020 12:38 am
DarkElvenAngel wrote:
Wed Sep 16, 2020 12:13 am
So looking at that if I try to define everything on the stack...
That is not what I meant about not using "new", "delete", and raw pointers.

It's more about using modern C++ and so called "smart pointers". "shared_ptr", "unique_ptr" and whatever.

Did I say there is a reason I don't want to use C++ anymore?
In the link I read about this, they say that if you use the C method for allocating memory malloc and free don't mix them with new and delete or bad things can happen.
(Visions for that scene in Ghostbusters "Dog's and cat's living together... Mass hysteria!")

I agree it's really a pain language to get working right. You have all sorts of power but it's all cobbled together and if you do one thing wrong it blows up in your face. That said I'm looking at C++ to be C with Classes and I will try not use any of C++ standard libraries if I can help it, they're Pandora's box probably a black hole inside.

So many movie quotes about power, responsibility, corruption ... But I digress

I would still like to make that a bit more efficient if I can.

Heater
Posts: 16515
Joined: Tue Jul 17, 2012 3:02 pm

Re: Advice on this code

Wed Sep 16, 2020 6:35 am

DarkElvenAngel wrote:
Wed Sep 16, 2020 1:51 am
I agree it's really a pain language to get working right. You have all sorts of power but it's all cobbled together and if you do one thing wrong it blows up in your face.
Ha!

The mathematician Po-Shen Loh said:
Whenever I have to program in C++ it feels like I'm trying to build a Formula 1 car using duct tape.
Memory in C++ is a leaky abstraction .

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Sat Sep 19, 2020 1:25 am

For anyone interested I figured out how to make adding new elements into a one liner

Code: Select all

class Window
{
    // PRIVATE DATA
    unsigned char   m_id;
    const char*     m_name;
    Element         m_children[16];
    int             m_index;
    public:
    Window(unsigned char id, const char* name) // constructor
        : m_id(id), m_name(name), m_index(0)
    {
        
    }

    void Add_Element (Element child)
    {
        m_children[m_index] = child;
        m_index++;
    }

    template <typename... Args>
    void New_Element (Args&&... args)
    {
        m_children[m_index] = Element(std::forward<Args>(args)...);
        m_index++;
        
    }

    const char* Get_Title()
    {
        return m_name;
    }

    int Get_Count ()
    {
        return m_index;
    }

    int Click(int x, int y)
    {

        for (int i = 0; i < m_index; i++)
        {
            if (m_children[i].HitBox(UIE_CLICK,x,y)) return i;
        }
        return -1;
    }

    int Event(UIE_EVENTS e, int x, int y)
    {

        for (int i = 0; i < m_index; i++)
        {
            if (m_children[i].HitBox(e,x,y)) return i;
        }
        return -1;
    }

    void Draw()
    {
        for (int i = 0; i < m_index; i++)
        {
            m_children[i].Draw(true);
        }
    }

    void Redraw()
    {
        for (int i = 0; i < m_index; i++)
        {
            m_children[i].Draw(false);
        }
    }

};

Code: Select all

// OLD WAY
Element btn8(&btn_CB, UIE_Button, 190,360,160,80,3, "Button 8");
page0.Add_Element(btn8);
// NEW WAY
page0.New_Element(&btn_CB, UIE_Button, 390,60,160,80,3, "Button 9");

I had to change from an array of pointers to an array of object however. I'm not sure if this is a problem it's not memory efficient I know that much. I could use dynamic memory allocation to make the impact less, like a vector class?? I wanted to avoid using standard libraries I could I needed to though since I'm not sure if I can get just the std::forward call on it's own.

swampdog
Posts: 402
Joined: Fri Dec 04, 2015 11:22 am

Re: Advice on this code

Sat Sep 19, 2020 4:23 pm

The ancient GEM GUI used by the Atari ST and early Mac was very lean. Horrible implementation due to hardware restraints but the design still stands up. It had the notion of an OBJECT in which was contained very little..

http://toshyp.atari.org/en/008016.html#OBJECT

Code: Select all

typedef struct
{
   int16_t    ob_next;   /* The next object               */
   int16_t    ob_head;   /* First child                   */
   int16_t    ob_tail;   /* Last child                    */
   uint16_t   ob_type;   /* Object type                   */
   uint16_t   ob_flags;  /* Manipulation flags            */
   uint16_t   ob_state;  /* Object status                 */
   void       *ob_spec;  /* More under object type        */
   int16_t    ob_x;      /* X-coordinate of the object    */
   int16_t    ob_y;      /* Y-coordinate of the object    */
   int16_t    ob_width;  /* Width of the object           */
   int16_t    ob_height; /* Height of the object          */
} OBJECT;
It's a tree structure. ob_x,ob_y are not absolute co-ordinates but rather offsets from its parent node which means you only need adjust ob_x,ob_y of a parent node and issue a redraw to visually adjust the positioning of part of the tree. Consequently the entire visual tree can be moved simply by changing ob_x,ob_y of the root node and a redraw. The actual redraw command was reasonably trivial: objc_draw() is passed little more than an OBJECT pointer and a clipping rectangle.

The ob_next,ob_head,ob_tail "pointers" weren't real pointers. They were trying to save memory. All the GUI stuff for a process/app had to fit into 32k originally thus the whole thing was loaded as an array and the values were merely array indices. The glaring omission of an ob_prev shows just how tight things were.

Off the top of my head, you could implement this now as..

Code: Select all

struct OBJECT {
 size_t	ob_prev,
		ob_next;
 size_t	ob_head,
 		ob_tail;
 int		ob_x,
 		ob_y;
 size_t	ob_w,
 		ob_h;
//other stuff
};
..reworking it a bit (pseudo code) as..

Code: Select all

struct CoOrd {
int x,y;
};

struct CoArea {
size_t w,h;
};

struct OBJECT {
 size_t	ob_prev,
		ob_next;
 size_t	ob_head,
 		ob_tail;
 CoOrd	off;
 CoArea	area;
};
..maybe do something with the indices..

Code: Select all

struct Object {
 dListItem	pn;
 ChildItem	ht;
 CoOrd	off;
 CoArea	area;
};
..though that might be taking it too far.

You could hold Objects in a deque<> and like GEM, just use array indexes. Now you have no pointers (atm) and deque<> allows insertion removal of items. Your objc_draw() just needs to walk the tree in a defined manner: GEM did it root, head descent, walking left to right. Doesn't matter as long as it is consistent. Maybe have a callback function to visually draw the item. I'm thinking on the hoof here: I might add in a "pointer" to the parent also.

The advantage of the above is no graphics are required at this stage. All your tree and linked list traversal code can be written & tested without drawing a thing.

Another thing Gem did was have a "message pump". Simply a FIFO buffer of events. When the application was ready it would pull the next message out of the buffer (event_multi() if you want to look it up) and act on it. This allows separation. A completely distinct process (back then an interrupt handler now it would be a thread or simply your own app) can inject mouse down/up, movement, keyboard events into the FIFO. Again this can be worked on its own. Some code to distinguish against mouse down/up/double click and all you need to test is the correct message gets into the FIFO.

DarkElvenAngel
Posts: 978
Joined: Tue Mar 20, 2018 9:53 pm

Re: Advice on this code

Sat Sep 19, 2020 7:19 pm

swampdog,

That's some really interesting information! I didn't consider looking at some of the old 8 bit micro's for code ideas I had no idea there was even published code for them in C no less. I though it would all be written in assembly.

I'm not going to pretend I understand what this means just yet
You could hold Objects in a deque<> and like GEM, just use array indexes. Now you have no pointers (atm) and deque<> allows insertion removal of items.
I've seen this in code and I haven't played it much. I only just figured out how to work the templates into my code. Okay wait I looked it up and I tried to put this in and I had template errors. I get why I had them but I didn't bother to fix it. Now I see this is going back to my idea of the vector class. I figured out the solution for the one line entries from watch how to write your own vector class. I'll have to work through the rest of that.

This is really great there is so much here.
inprog002.png
inprog002.png (10.82 KiB) Viewed 434 times
I've got the drawing routines running here's an in progress screenshot, the buttons are purple I'm using a very simple 16 colour pallet. Retro code for a retro look I love it!

Heater
Posts: 16515
Joined: Tue Jul 17, 2012 3:02 pm

Re: Advice on this code

Sat Sep 19, 2020 7:30 pm

GEM was never on a Mac. In fact Apple Computer sued Digital Research on claims of infringing on Apple's look an feel. That is how good GEM was. GEM was better than early versions of Windows.
Memory in C++ is a leaky abstraction .

swampdog
Posts: 402
Joined: Fri Dec 04, 2015 11:22 am

Re: Advice on this code

Sat Sep 19, 2020 7:47 pm

Heater wrote:
Sat Sep 19, 2020 7:30 pm
GEM was never on a Mac. In fact Apple Computer sued Digital Research on claims of infringing on Apple's look an feel. That is how good GEM was. GEM was better than early versions of Windows.
My bad. Forgot that over the years. Not quite sure how because I knew about the legal battle! It was early PC's where GEM made an appearance.

Return to “C/C++”