1

I trying to create a function to transpose in-place a bitmap. But so far, the result I get is all messed up, and I can’t find what I’m doing wrong.

Source bitmaps are as a 1d pixel array in ARGB format.

void transpose(uint8_t* buffer, const uint32_t width, const uint32_t height)
{
    const size_t stride = width * sizeof(uint32_t);

    for (uint32_t i = 0; i < height; i++)
    {
        uint32_t* row = (uint32_t*)(buffer + (stride * i));
        uint8_t* section = buffer + (i * sizeof(uint32_t));

        for (uint32_t j = i + 1; j < height; j++)
        {
            const uint32_t tmp = row[j];
            row[j] = *((uint32_t*)(section + (stride * j)));
            *((uint32_t*)(section + (stride * j))) = tmp;
        }
    }
}

enter image description here

UPDATE:

To clarify and avoid confusions as it seems some people think this is just a rotate image question. Transposing an image is composed by 2 transformations: 1) flip horizontally 2) Rotate by 90 CCW. (As shown in the image example, see the arrow directions)

PerracoLabs
  • 16,449
  • 15
  • 74
  • 127
  • your example has `height != widht`, so not exactly clear what you mean with in-place – 463035818_is_not_an_ai Oct 16 '18 at 12:42
  • By in-place I mean not copying the result into a different bitmap, neither allocating a temporary working buffer with the same dimensions as the source. So, to transform directly the input bitmap buffer. – PerracoLabs Oct 16 '18 at 12:46
  • it would be best to spin out the per-pixel address calculation into a small inline function to reduce the amout of math going on –  Oct 16 '18 at 12:56
  • I take my previous comment about stride back, you confused me by going back and forth between uint32_t* and uint8_t* –  Oct 16 '18 at 13:04
  • now, does it work correctly for square bitmaps? –  Oct 16 '18 at 13:06
  • Not it doesn't because the implementation I'm doing is without aspect ratio limitations. So being the bitmap square or not will not make any difference. – PerracoLabs Oct 16 '18 at 13:10
  • I would suggest a) do a single cast to a `uint32_t` pointer - that will simplify your code enormously. b) search for and read "How to debug small programs" by Eric Lippert c) single step your algorithm for a simple 2x3 bitmap - either in a debugger, or on paper. – Martin Bonner supports Monica Oct 16 '18 at 13:14
  • Maybe we need some extra context, does the bitmap buffer have some dimensions of its own? Or does it match the dimensions of the bitmap? After flipping the contents, do you leave width and height as they were or do you exchange them as well? –  Oct 16 '18 at 13:14
  • @jakub_d, The buffer dimensions are specified in the function parameters, width and height, and they match with it. When the transformation is completed the function caller knows that they have to be swapped. As you can see in the example image, the result has the dimensions correctly swapped. – PerracoLabs Oct 16 '18 at 13:27
  • so... the output has height and width swapped, so the address calculation for pixel[i][j] is different for the input and output? –  Oct 16 '18 at 13:36
  • The image is never accessed as a 2d array (pixel[i][j]), The input is always a 1d pixel array. – PerracoLabs Oct 16 '18 at 13:42
  • if you imagine accessing it as 2d like my addr example –  Oct 16 '18 at 13:46
  • 1
    Possible duplicate of [Algorithm to rotate an image 90 degrees in place? (No extra memory)](https://stackoverflow.com/questions/2968397/algorithm-to-rotate-an-image-90-degrees-in-place-no-extra-memory) – Alan Birtles Oct 16 '18 at 13:56
  • @Alan not duplicate. Transposing is to mirror horizontal + rotate 90 CCW, the link provided doesn't give a solution. – PerracoLabs Oct 16 '18 at 14:05
  • The accepted answer does mention doing a transpose then a mirror, the wikipedia article linked gives you the required algorithm: https://en.wikipedia.org/wiki/In-place_matrix_transposition#Non-square_matrices:_Following_the_cycles – Alan Birtles Oct 16 '18 at 14:08
  • @Alan. Thanks for the links. Jut to clarify that transposing is a transformation made by both mirror and rotation. My question is not about how to rotate an image, but to perform an in-place transpose which implies both actions, and not only rotation. But thanks for the links, I'll have a look. – PerracoLabs Oct 16 '18 at 14:17

3 Answers3

1

I think the problem is more complex than you realise and is not simply a case of swapping the pixels at x, y with the pixels at y, x. If you consider a 3*7 pixel image in which I've labelled the pixels a-u:

abcdefg
hijklmn
opqrstu

Rotating this image gives:

aho
bip
cjq
dkr
els
fmt
gnu

Turning both images into a 1D array gives:

abcdefghijklmnopqrstu

ahobipcjqdkrelsfmtgnu

Notice that b has moved to the position of d but has been replaced by h.

Rethink your algorithm, draw it out for a small image and make sure it works before attempting to implement it.

Due to the complexity of the task it may actually end up being faster to create a temporary buffer, rotate into that buffer then copy back as it could end up with fewer copies (2 per pixel) than the inplace algorithm that you come up with.

Alan Birtles
  • 32,622
  • 4
  • 31
  • 60
  • its also not a matter of just swapping element at `x,y` with an element at some other position `a,b`. Actually I didnt know that transposing a matrix is that complicated – 463035818_is_not_an_ai Oct 16 '18 at 19:14
0

Mostly equivalent code that should be easier to debug:

inline uint32_t * addr(uint8_t* buffer, const uint32_t width, uint32_t i, uint32_t j) {
    uint32_t * tmp = buffer;
    return tmp+i*width+j;
}

void transpose(uint8_t* buffer, const uint32_t width, const uint32_t height) {
    for (uint32_t i = 0; i < min(width,height); i++) {
        for (uint32_t j = 0; j < i; j++) {
            uint32_t * a = addr(buffer, width, i, j);
            uint32_t * b = addr(buffer, width, j, i);
            const uint32_t tmp = *a;
            *a = *b;
            *b = tmp;
        }
    }
}

If this doesn't work right, it is possible that it needs to know not just the width of the picture, but also the width of the underlying buffer. This only flips the square portion at the top-left, more work would be needed for non-square bitmaps. (or just pad everything to square before using...)

  • seems form the comments that OP is trying to change the dimensions of the bitmap at the same time, so this will not work –  Oct 16 '18 at 13:42
  • The algorithm doesn't try to change the dimensions. This is a 1d pixel array. Transposing an image is: 1) flip horizontally, 2) rotate by 90 CCW. So as the input is 1d pixel array, caller functions know that after transposing, both Width and Height have to be treated as swapped because of the rotation. – PerracoLabs Oct 16 '18 at 13:51
  • If you take the result, consider a vertical line using height * width addressing, what coordinates correspond to those memory addresses in the source using flipped width * height addressing? You get an angled line, not a vertical. Which is going to make it difficult to determine what should be swapped with what. –  Oct 16 '18 at 14:11
0

Note that transposing a matrix in place is not trivial when N!=M. See eg here for details.

The reason is that when N=M you can simply iterate through half of the matrix and swap elements. When N!=M this isnt the case.

For illustration, consider a simpler case:

First a 2d view on 1d data:

struct my2dview {
    std::vector<int>& data;
    int width,height;
    my2dview(std::vector<int>& data,int width,int height):data(data),width(width),height(height){}
    int operator()(int x,int y) const { return data[x*width + y]; }
    int& operator()(int x,int y){ return data[x*width + y]; }
    my2dview get_transposed() { return my2dview(data,height,width);}
};


std::ostream& operator<<(std::ostream& out, const my2dview& x){
    for (int h=0;h<x.height;++h){
        for (int w=0;w<x.width;++w){
            out << x(h,w) << " ";
        }
        out << "\n";
    }
    return out;
}

Now a transpose that would work for N=M:

my2dview broken_transpose(my2dview x){
    auto res = x.get_transposed();
    for (int i=0;i<x.height;++i){
        for (int j=0;j<x.width;++j){
            res(j,i) = x(i,j);
        }
    }
    return res;
}

Using it for some small matrix

int main() {
    std::vector<int> x{1,2,3,4,5,6};
    auto v = my2dview(x,2,3);
    std::cout << v << '\n';
    std::cout << v.get_transposed() << '\n';
    auto v2 = broken_transpose(v);
    std::cout << v2;
}

prints

1 2
3 4
5 6

1 2 3
4 5 6

1 3 2
2 2 6

Conclusion: The naive swapping elements approach does not work for non-square matrices.

Actually this answer just rephrases the one by @Alan Birtles. I felt challenged by his

Due to the complexity of the task it may actually end up being faster to create a temporary buffer [...]

just to come to the same conclusion ;).

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185