1

I have the following two objects. Im wondering if there is a way to have Pixel as a base class of PixelBGR so that any operator (+,-,*,/, [], etc.) could be used without redefining them ?

template<class T, std::size_t N>
struct Pixel
{
    T ch[N];

    inline T& operator[](const int x)
    {
        return ch[x];
    }
};


template<class T>
struct PixelBGR
{
    union
    {
        struct
        {
            T b;
            T g;
            T r;
        };
        T ch[3];
    };

    inline T& operator[](const int x)
    {
        return ch[x];
    }
};

EDIT: As suggested by πάντα ῥεῖ, here more details about what Im trying to do.

Im trying to have a generic class Pixel, which will be template to handle any type or size.

The usual are 1,2,3,4,8 or 16. The class with defines some operator such as +,-,*, etc.

Since most of the time, the Pixel<T,3> is a BGR pixel, I would like to define rapid access to r,g and b to avoid confusion, but still store it as BGR.

But the derived class should also provide the Operator which will be generic based on N.

EDIT2: By reading the comment of SergeyA, I forgot to say that the struct Pixel must not change size.

So I think balki answer is the best, by using member function. I was trying to make it with variables to avoid too much char ie: adding the (), but it seems to be too complicated for nothing. I still investigating CRTP, but I dont get it well, Im reading on that.

timrau
  • 22,578
  • 4
  • 51
  • 64
Vuwox
  • 2,331
  • 18
  • 33
  • 7
    FWIW, if you are using the union to be able to do `PixelBGR.r = 30` and have `PixelBGR[2]` access that value then you'll be in UB land. C++ does not allow that kind of access. (it will often "work", but it is not standard conforming code) – NathanOliver Apr 01 '19 at 19:30
  • It looks like you're going to enter the land of _undefined behavior_. Be careful about every step done in that swamp. – πάντα ῥεῖ Apr 01 '19 at 19:30
  • Oh! Thank, is there a better way to do that ? with having `undefined behavior` ? – Vuwox Apr 01 '19 at 19:31
  • One *can* derive from template arguments. – Jesper Juhl Apr 01 '19 at 19:32
  • You can just write partial specialization for `Pixel` with `N = 3` – user7860670 Apr 01 '19 at 19:34
  • @Vuwox _"Oh! Thank, is there a better way to do that ? with having undefined behavior ?"_ Probably answerable if you elaborate about what you actually want to achieve, and what are your concrete requirements. Getting a `union` right is tightly bound to either pure standard c++ or knowing a 200% that all your casts would work well with your specific environment. – πάντα ῥεῖ Apr 01 '19 at 19:35
  • One way to do without undefined behavior: https://gcc.godbolt.org/z/9Fo1m6 – balki Apr 01 '19 at 19:40
  • To expand @JesperJuhl's comment a little bit: [What is the curiously recurring template pattern (CRTP)?](https://stackoverflow.com/questions/4173254/what-is-the-curiously-recurring-template-pattern-crtp) – πάντα ῥεῖ Apr 01 '19 at 19:42
  • You can named have references and point them to the array elements. – SergeyA Apr 01 '19 at 19:42
  • @πάνταῥεῖ I elaborated my question. – Vuwox Apr 01 '19 at 19:43
  • Hope this helps: https://gcc.godbolt.org/z/uQqele – balki Apr 01 '19 at 19:50
  • Thanks to all! I was able to produce a code that respect my need using CRTP. Feel free to evaluate the code and point where it could be problematic, thank again! – Vuwox Apr 02 '19 at 14:15

3 Answers3

1

Answering the question as asked, this should give OP reuse of the operators without any undefined behavior:

#include <cstddef>

template<class T, std::size_t N>
struct Pixel
{
    T ch[N];

    inline T& operator[](const int x)
    {
        return ch[x];
    }
    Pixel& operator+= (const Pixel& ) { return *this;}

};

template<class T, std::size_t N>
Pixel<T, N> operator+ (const Pixel<T, N>& l, const Pixel<T, N>& r);

template<class T>
struct BgrPixel : Pixel<T, 3> {
    using base = Pixel<T, 3>;
    using base::base;
    BgrPixel(const base& b) : base(b) { };
    T& b = base::ch[0];
    T& g = base::ch[1];
    T& r = base::ch[2];
};


BgrPixel<int> a, b;

BgrPixel<int> c = a + b;

Alterinative would be to have b(), g() and r() as a member functions, but this would require you to access them as functions. You would also need const and non-const versions of them.

The benefits, however, would be that the size of the struct will not be increased and copy assignment would work naturally (which, in turn, could be solved by providing custom copy assignment).

SergeyA
  • 61,605
  • 5
  • 78
  • 137
  • 1
    It is better to have `b`, `g`, `r` as member functions so that it does not increase the size of pixel. e.g. https://gcc.godbolt.org/z/1jAJ1v – balki Apr 01 '19 at 19:56
  • And doesn't "break" copy assignment. – Jarod42 Apr 01 '19 at 19:58
  • I think balki answer seems good, it does what I want, without having the undefined behavior from the union I tried before. – Vuwox Apr 01 '19 at 20:05
  • @Jarod42 for copy assignment I would just define my own copy assignment operator. – SergeyA Apr 01 '19 at 20:06
  • @Vuwox balki's answer doesn't have solution for operators reuse :). – SergeyA Apr 01 '19 at 20:07
  • You right. I just tested it, and I would have to redefine my operator. Question still on then... I still trying on my side. – Vuwox Apr 01 '19 at 20:23
  • @Vuwox my answer should cover that, why don't you try it? – SergeyA Apr 01 '19 at 20:28
  • @SergeyA Sorry, I updated my question. The problem is that I need the size to be fix. Since I need to be able to cast PixelBGR over an array of byte that store the data. So I can't have extra byte during reinterpretation, so same size is mandatory also. Sorry... I forgot on the first edition. – Vuwox Apr 01 '19 at 20:33
  • What do you mean by "not change size"? `Pixel` is still the same size. – Acorn Apr 02 '19 at 07:23
  • @Acorn `Pixel` is same size as allocation. But `BgrPixel` is not. – Vuwox Jun 14 '19 at 21:43
  • @Acorn Cause of the variable R,G,B created. But we could remove them and replace it by member function `r()`, `g()` and `b()`. – Vuwox Jun 14 '19 at 21:46
1

The Curiously Recurring Template Pattern (CRTP) would work well in this case. In the CRTP the Derived class is used as a template argument to the Base class. Chapter 16.3 The Curiously Recurring Template Pattern (CRTP), from the C++ Templates - The Complete Guide, by David Vandevoorde and Nicolai M. Josuttis, explains things in more detail.

From the comments below, the usage of a union{struct{...}...} causes undefined behaviour (UB), but there have been some contradicting opinions upon this. As far as I'm aware, it is a gnu extension and supported by almost every compiler. glm for example uses union-structs quite very often.

As an alternative approach, you can use aliases (references) for the r,g,b variables.

#include <iostream>

template<typename T, std::size_t N, template<typename,std::size_t> class B >
struct Pixel
{
    B<T,N> *crtp = static_cast<B<T,N>*>(this);

    T& operator[](std::size_t x)
    {
        return crtp->ch[x];
    }

    Pixel& operator = (const Pixel &t)
    {
        crtp->ch[0] = t.crtp->ch[0];
        crtp->ch[1] = t.crtp->ch[1];
        crtp->ch[2] = t.crtp->ch[2];
        return *crtp;
    }

    B<T,N> operator + (const B<T,N> &t)
    {
        B<T,N> tmp;
        tmp[0] = crtp->ch[0] + t.crtp->ch[0];
        tmp[1] = crtp->ch[1] + t.crtp->ch[1];
        tmp[2] = crtp->ch[2] + t.crtp->ch[2];
        return tmp;
    }

    B<T,N> operator - (const B<T,N> &t)
    {
        B<T,N> tmp;
        tmp[0] = crtp->ch[0] - t.crtp->ch[0];
        tmp[1] = crtp->ch[1] - t.crtp->ch[1];
        tmp[2] = crtp->ch[2] - t.crtp->ch[2];
        return tmp;
    }
};

template<typename T, std::size_t N=3>
struct PixelBGR : Pixel<T, N, PixelBGR>
{
    T ch[3];
    T &r;
    T &g;
    T &b;

    PixelBGR() : ch{},r(ch[0]),g(ch[1]),b(ch[2])
    {}
    PixelBGR& operator = (const PixelBGR &p)
    {
        ch[0] = p.ch[0];
        ch[1] = p.ch[1];
        ch[2] = p.ch[2];
        return *this;
    }
};

int main()
{
    PixelBGR<int> p;

    p.r = 25;
    p.g = 14;
    p.b = 58;

    std::cout<< p[0] <<" , "<<p[1]<<" , "<<p[2] <<std::endl;

    PixelBGR<int> q;
    q = p;
    std::cout<< q[0] <<" , "<<q[1]<<" , "<<q[2] <<std::endl;

    PixelBGR<int> res1;
    res1 = q + p;
    std::cout<< res1.r <<" , "<<res1.g<<" , "<<res1.b <<std::endl;

    PixelBGR<int> res2;
    res2 = q - p;
    std::cout<< res2.r <<" , "<<res2.g<<" , "<<res2.b <<std::endl;
}

Result:

25 , 14 , 58
25 , 14 , 58
50 , 28 , 116
0 , 0 , 0

Example using references: https://rextester.com/AZWG4319

Example using union-struct: https://rextester.com/EACC87146

Constantinos Glynos
  • 2,952
  • 2
  • 14
  • 32
  • What the difference between my solution ? Why this is not an *undefined behavior* ? I never heard about CRTP before, I read about it this afternoon, but not enough to understand it well yet. – Vuwox Apr 02 '19 at 00:35
  • It is undefined behavior. – Acorn Apr 02 '19 at 07:21
  • @Vuwox: In the example above, all commonly used functions are part of the base class and used in the derived class without the need of re-implementing them. CRTP stands for Curiously Recuring Template Pattern and it is such a case where the Derived class is used as a template argument to the Base class. I would highly recommend you read chapter 16.3 The Curiously Recurring Template Pattern (CRTP), from the C++ Templates - The Complete Guide, by David Vandevoorde and Nicolai M. Josuttis. There is 2nd edition released for C++11 and higher, but I haven't read that book. – Constantinos Glynos Apr 02 '19 at 09:00
  • @Acorn: How is it UB?? Please elaborate with facts before throwing out comments like that. – Constantinos Glynos Apr 02 '19 at 09:02
  • @ConstantinosGlynos It has already been explained: you cannot write to a member of a `union` and then inspect inactive members. See [class.union]p1 of the C++17 standard. – Acorn Apr 02 '19 at 09:13
  • @Acorn: `union{struct{...}..}` have been used for year in games. However there are some very contradicting opinions out there and I don't want to say anyting wrong. Thanks for the standard reference. I look into it asap. I guess OP can use aliases instead of a union-struct, but in the example above, that would require to re-implement the copy-assignment operator. Example: https://rextester.com/AZWG4319 – Constantinos Glynos Apr 02 '19 at 09:36
  • 1
    @ConstantinosGlynos One thing is what compilers do in order to avoid breaking people's code (or simply because their optimizer does not take advantage of this) and another thing is what the standard says. Also, note that this is different in C. There, this particular usage would be fine. – Acorn Apr 02 '19 at 09:51
  • 1
    @Acorn: I'll amend the answer to using aliases until I get dive into the `union{struct{..}..}` thing. To be fair, I've seen this in C code. So, maybe it was simply dragged along from C to C++ without much investigation. Interesting..... – Constantinos Glynos Apr 02 '19 at 10:02
  • @ConstantinosGlynos Yep, there is a lot of code out there that uses this pattern (and others) in C++, so it is not a surprise. Sometimes even written by people that have full knowledge of the standard rules. It depends on your goals and your environment. Your compiler may very well happen to do what you want, or even explicitly "support" this. – Acorn Apr 02 '19 at 10:33
  • @Acorn: Thanks for the advice and the standard ref! I'll be sure to pop the hood on this one. – Constantinos Glynos Apr 02 '19 at 10:36
  • @ConstantinosGlynos Thanks for your code, I learn about CRTP a bit more, and I was able to create what I needed. – Vuwox Apr 02 '19 at 14:13
0

First thanks to all of you for advise, and special thanks to @Constantinos Glynos, @balki and @SergeyA for the example they provide, those help me to achieve a solution that match my need.

I implemented the BGR and BGRA to show that the N works fine, now I just need to implement all the operator, and functions that I require.

Please feel free to edit, or tell me if there is something wrong with this.

Pixel.h

#include <cstdio>   // std::size_t
#include <iostream>     // std::cout


template<typename T, std::size_t N, template<typename, std::size_t> class B >
struct Pixel
{
    T ch[N];

    // ==============================================================
    // Overload the accessor (so .ch[0] == direct access with [0].
    T& operator[](std::size_t x){ return ch[x]; }

    // ==============================================================
    // Copy-assignement
    Pixel& operator=( const Pixel &t )
    {
        for ( int i = 0; i < N; i++ )
            ch[i] = t.ch[i];
        return *this;
    }

    // ==============================================================
    // Operator
    B<T, N> operator+( const B<T, N> &t )
    {
        B<T, N> tmp;
        for ( int i = 0; i < N; i++ )
            tmp[i] = ch[i] + t.ch[i];
        return tmp;
    }

    B<T, N> operator-( const B<T, N> &t )
    {
        B<T, N> tmp;
        for ( int i = 0; i < N; i++ )
            tmp[i] = ch[i] - t.ch[i];
        return tmp;
    }

    template<typename T, std::size_t N, template<typename, std::size_t> class B >
    friend std::ostream& operator<<( std::ostream& os, const Pixel &t );
};

// To print the vector
template<typename T, std::size_t N, template<typename, std::size_t> class B >
std::ostream& operator<<( std::ostream& os, const B<T, N> &t )
{
    os << "Pixel: (" << t.ch[0];
    for ( int i = 1; i < N; i++ )
        os << ", " << t.ch[i];
    os << ")";
    return os;
}




template<typename T, std::size_t N = 3>
struct BGR : Pixel<T, N, BGR>
{
    T& b() { return ch[0]; }
    T& g() { return ch[1]; }
    T& r() { return ch[2]; }
};


template<typename T, std::size_t N = 4>
struct BGRA : Pixel<T, N, BGRA>
{
    T& b() { return ch[0]; }
    T& g() { return ch[1]; }
    T& r() { return ch[2]; }
    T& a() { return ch[3]; }
};

Main.cpp

int main() {
    std::cout << "Sizeof a float BGR: " << sizeof(BGR<float>) << std::endl;
    std::cout << "Sizeof a float BGRA: " << sizeof(BGRA<float>) << std::endl;

    BGR<int> p;
    p.r() = 25;
    p.g() = 14;
    p.b() = 58;

    std::cout << p << std::endl;
    std::cout << p[0] << " , " << p[1] << " , " << p[2] << std::endl;
    std::cout << p.b() << " , " << p.g() << " , " << p.r() << std::endl;

    BGR<int> q;
    q = p;
    std::cout << q[0] << " , " << q[1] << " , " << q[2] << std::endl;

    BGR<int> res1;
    res1 = q + p;
    std::cout << res1.r() << " , " << res1.g() << " , " << res1.b() << std::endl;

    BGR<int> res2;
    res2 = q - p;
    std::cout << res2.r() << " , " << res2.g() << " , " << res2.b() << std::endl;

    BGRA<float> a;
    a.r() = 255.0f;
    a.g() = 0.0f;
    a.b() = 0.0f;
    a.a() = 128.5f;

    BGRA<float> b = a;
    std::cout << a << std::endl;

    return 0;
}
Vuwox
  • 2,331
  • 18
  • 33