Question about bits (debugging>

[JoeC]

I am working on a graphics program but my question has nothing to do
with graphics but trying to get an algorithm to work. I set graphics
from a 16x16 grid to bits of a graphic with:

bitData[index++] = binTemp[0] * 128 + binTemp[1] * 64 +
binTemp[2] * 32 + binTemp[3] * 16 +
binTemp[4] * 8 + binTemp[5] * 4 +
binTemp[6] * 2 + binTemp[7];

But I want to populate that same pad from bits:


void populate(int num){
int bits[8] = {0,0,0,0,0,0,0,0};
int counter = 0;
int placer = 0;
BYTE temp;
clear();
std::vector<BYTE> vb = work.getBits();
std::vector<BYTE>::const_iterator itr = vb.begin();
for(int dwn = 0; dwn < work.getPixels(); dwn++){
for(int acc = 0; acc < 2; acc++){
if(acc == (num) || acc == (num+1)){
temp = *itr;
if(temp >= 128){
bits[0] = 1;
temp=-128;
}
if(temp >= 64){
bits[1] =1;
temp-=64;
}
if(temp >= 32){
bits[2] =1;
temp-=32;
}
if(temp >= 16){
bits[3] =1;
temp-=16;
}
if(temp >= 8){
bits[4] =1;
temp-=8;
}
if(temp >= 4){
bits[5] =1;
temp-=4;
}
if(temp >= 2){
bits[6] =1;
temp-=2;
}
if(temp >= 1){
bits[7] =1;
}
table[0+(counter*8)] = bits[0];
table[1+(counter*8)] = bits[1];
table[2+(counter*8)] = bits[2];
table[3+(counter*8)] = bits[3];
table[4+(counter*8)] = bits[4];
table[5+(counter*8)] = bits[5];
table[6+(counter*8)] = bits[6];
table[7+(counter*8)] = bits[7];
for(int l = 0; l != 7; l++){bits[l]=0;}
counter++;
itr++;
}
}
}
}

It does not work. It messes up if the number is over 128 that is the
first bit 10000000 is above 128. Anything thing look wrong. I will
send my program or post more code if that is needed.

You can see a functioning version of my program at:
http://www.planetsourcecode.com/vb/scripts/ShowCode.asp?txtCodeId=11996&lngWId=3

 

[Jim Langston]

I am working on a graphics program but my question has nothing to do
with graphics but trying to get an algorithm to work. I set graphics
from a 16x16 grid to bits of a graphic with:

bitData[index++] = binTemp[0] * 128 + binTemp[1] * 64 +
binTemp[2] * 32 + binTemp[3] * 16 +
binTemp[4] * 8 + binTemp[5] * 4 +
binTemp[6] * 2 + binTemp[7];

But I want to populate that same pad from bits:


void populate(int num){
int bits[8] = {0,0,0,0,0,0,0,0};
int counter = 0;
int placer = 0;
BYTE temp;
clear();
std::vector<BYTE> vb = work.getBits();
std::vector<BYTE>::const_iterator itr = vb.begin();
for(int dwn = 0; dwn < work.getPixels(); dwn++){
for(int acc = 0; acc < 2; acc++){
if(acc == (num) || acc == (num+1)){
temp = *itr;
if(temp >= 128){
bits[0] = 1;
temp=-128;
}
if(temp >= 64){
bits[1] =1;
temp-=64;
}
if(temp >= 32){
bits[2] =1;
temp-=32;
}
if(temp >= 16){
bits[3] =1;
temp-=16;
}
if(temp >= 8){
bits[4] =1;
temp-=8;
}
if(temp >= 4){
bits[5] =1;
temp-=4;
}
if(temp >= 2){
bits[6] =1;
temp-=2;
}
if(temp >= 1){
bits[7] =1;
}
table[0+(counter*8)] = bits[0];
table[1+(counter*8)] = bits[1];
table[2+(counter*8)] = bits[2];
table[3+(counter*8)] = bits[3];
table[4+(counter*8)] = bits[4];
table[5+(counter*8)] = bits[5];
table[6+(counter*8)] = bits[6];
table[7+(counter*8)] = bits[7];
for(int l = 0; l != 7; l++){bits[l]=0;}
counter++;
itr++;
}
}
}
}

It does not work. It messes up if the number is over 128 that is the
first bit 10000000 is above 128. Anything thing look wrong. I will
send my program or post more code if that is needed.

You can see a functioning version of my program at:
http://www.planetsourcecode.com/vb/scripts/ShowCode.asp?txtCodeId=11996&lngWId=3

You state it messes up if the number is over 128. Are you aware of the sign
bit? For a signed value, such as signed char, int, the high bit is used to
represent negative. If BYTE is char or signed char, then any value with the
high bit set will be negative.

I.E. 11111111 is -1. 11111110 is -2 using 2's compliment.

How is BYTE defined? Try making sure it's unsigned char and see if it then
works.

 

[Jerry Coffin]

[ ... ]

temp=-128;

My guess is that this isn't what you intended. I'd guess you meant '-='
instead of '=-'.

Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical, and so
long ago I doubt it ever made it into C++ at all.

 

[Andy Champ]

[ ... ]

temp=-128;

My guess is that this isn't what you intended. I'd guess you meant '-='
instead of '=-'.

Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical, and so
long ago I doubt it ever made it into C++ at all.

I think you're right.

But might I suggest using

bits[0] = (temp & 128) ? 1 : 0;

in place of

if(temp >= 128){
bits[0] = 1;
temp=-128;
}

It'll give the optimiser more chance to work.

Andy

 

[Victor Bazarov]

[ ... ]

temp=-128;

My guess is that this isn't what you intended. I'd guess you meant
'-=' instead of '=-'.

Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical, and
so long ago I doubt it ever made it into C++ at all.

I think you're right.

But might I suggest using

bits[0] = (temp & 128) ? 1 : 0;

in place of

if(temp >= 128){
bits[0] = 1;
temp=-128;
}

It'll give the optimiser more chance to work.

Doesn't look equivalent. The value of 'temp' doesn't change in the
expression case, and it does in the 'if' case.

V

 

[Ian Collins]

[ ... ]

temp=-128;
My guess is that this isn't what you intended. I'd guess you meant
'-=' instead of '=-'.

Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical, and
so long ago I doubt it ever made it into C++ at all.

I think you're right.

But might I suggest using

bits[0] = (temp & 128) ? 1 : 0;

in place of

if(temp >= 128){
bits[0] = 1;
temp=-128;
}

It'll give the optimiser more chance to work.

Doesn't look equivalent. The value of 'temp' doesn't change in the
expression case, and it does in the 'if' case.

I was just about to post the same response when I realised the suggested
form does not require temp to be updated, all the bits can be tested in
place.

 

[Victor Bazarov]

Jerry Coffin wrote:
[ ... ]

temp=-128;
My guess is that this isn't what you intended. I'd guess you meant
'-=' instead of '=-'.

Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical,
and so long ago I doubt it ever made it into C++ at all.

I think you're right.

But might I suggest using

bits[0] = (temp & 128) ? 1 : 0;

in place of

if(temp >= 128){
bits[0] = 1;
temp=-128;
}

It'll give the optimiser more chance to work.

Doesn't look equivalent. The value of 'temp' doesn't change in the
expression case, and it does in the 'if' case.

I was just about to post the same response when I realised the
suggested form does not require temp to be updated, all the bits can
be tested in place.

I agree; my reply was out of context.

V

 

[JoeC]

Andy Champ wrote:
Jerry Coffin wrote:

[ ... ]
temp=-128;
My guess is that this isn't what you intended. I'd guess you meant
'-=' instead of '=-'.
Interestingly enough, at one time in the history of C, this would
actually have done the same thing, but that's purely historical,
and so long ago I doubt it ever made it into C++ at all.
I think you're right.
But might I suggest using
bits[0] = (temp & 128) ? 1 : 0;
in place of
if(temp >= 128){
bits[0] = 1;
temp=-128;
}
It'll give the optimiser more chance to work.
Doesn't look equivalent. The value of 'temp' doesn't change in the
expression case, and it does in the 'if' case.

I was just about to post the same response when I realised the
suggested form does not require temp to be updated, all the bits can
be tested in place.

I agree; my reply was out of context.

V

Thanks all for the help:
if(temp >= 128){
bits[0] = 1;
temp-=128;
did come out better, it is always good to have anther set of eyes.
Still, I am not sure how I could do this different. The suggestions
are good.

It does work better it is just the first bit is always on or always
off depending on how I do the last statment:

if(temp >= 2){
bits[6] =1;
temp-=2;
}
if(temp >= 1){
bits[7] =1;
}

I do have some more issues with the program but I will try to resolve
them before I ask for help.

 

[Alf P. Steinbach]

* JoeC:

> It does not work. It messes up if the number is over 128 that is the
first bit 10000000 is above 128. Anything thing look wrong. I will
send my program or post more code if that is needed.

As others have already mentioned else-thread, signedness is important (cast to
unsigned to extract bits).

For the bit extraction I suggest using std::bitset.

It will probably not be more efficient than your current code, but easier to get
right, and much shorter code.


Cheers, & hth.,

- Alf


PS: for efficient bitmapped graphics operations you'll probably benefit from
looking at so called "blitter" or "bitblt" operations. Unfortunately not part
of current standard C++ library. Would have been nice to have there.

 

[Jerry Coffin]

[ ... ]

A few more comments on your code:

if(temp >= 128){
bits[0] = 1;
temp=-128;
}
if(temp >= 64){
bits[1] =1;
temp-=64;
}
if(temp >= 32){
bits[2] =1;
temp-=32;
}
if(temp >= 16){
bits[3] =1;
temp-=16;
}
if(temp >= 8){
bits[4] =1;
temp-=8;
}
if(temp >= 4){
bits[5] =1;
temp-=4;
}
if(temp >= 2){
bits[6] =1;
temp-=2;
}
if(temp >= 1){
bits[7] =1;
}

I should have pointed out that I think there are better ways to do this.
One possibility using the ternary operator has already been pointed out.
Here's another possible alternative:

bits[0] = (temp >> 7) & 1;
bits[1] = (temp >> 6) & 1;
bits[2] = (temp >> 5) & 1;
bits[3] = (temp >> 4) & 1;
bits[4] = (temp >> 3) & 1;
bits[5] = (temp >> 2) & 1;
bits[6] = (temp >> 1) & 1;
bits[7] = (temp >> 0) & 1;

Of course, turning that into a loop is decidedly trivial.

table[0+(counter*8)] = bits[0];
table[1+(counter*8)] = bits[1];
table[2+(counter*8)] = bits[2];
table[3+(counter*8)] = bits[3];
table[4+(counter*8)] = bits[4];
table[5+(counter*8)] = bits[5];
table[6+(counter*8)] = bits[6];
table[7+(counter*8)] = bits[7];

You could also combine the two steps, moving data directly from temp to
table, without using bits[] at all.

for(int l = 0; l != 7; l++){bits[l]=0;}

I'd suggest that you never use 'l' as a variable name again. In many
fonts, it's quite easy to mistake for a '1', hurting readability a great
deal.

 

[Jerry Coffin]

[ ... ]

Here's another possible alternative:

bits[0] = (temp >> 7) & 1;
bits[1] = (temp >> 6) & 1;
bits[2] = (temp >> 5) & 1;
bits[3] = (temp >> 4) & 1;
bits[4] = (temp >> 3) & 1;
bits[5] = (temp >> 2) & 1;
bits[6] = (temp >> 1) & 1;
bits[7] = (temp >> 0) & 1;

I should have added that if temp is negative, this could (at least
theoretically) give incorrect results. In reality, it normally won't,
but if there's any chance of its being negative, converting to unsigned
(or starting with an unsigned type) wouldn't hurt.

 

[JoeC]

[ ... ]

A few more comments on your code:

if(temp >= 128){
bits[0] = 1;
temp=-128;
}
if(temp >= 64){
bits[1] =1;
temp-=64;
}
if(temp >= 32){
bits[2] =1;
temp-=32;
}
if(temp >= 16){
bits[3] =1;
temp-=16;
}
if(temp >= 8){
bits[4] =1;
temp-=8;
}
if(temp >= 4){
bits[5] =1;
temp-=4;
}
if(temp >= 2){
bits[6] =1;
temp-=2;
}
if(temp >= 1){
bits[7] =1;
}

I should have pointed out that I think there are better ways to do this.
One possibility using the ternary operator has already been pointed out.
Here's another possible alternative:

bits[0] = (temp >> 7) & 1;
bits[1] = (temp >> 6) & 1;
bits[2] = (temp >> 5) & 1;
bits[3] = (temp >> 4) & 1;
bits[4] = (temp >> 3) & 1;
bits[5] = (temp >> 2) & 1;
bits[6] = (temp >> 1) & 1;
bits[7] = (temp >> 0) & 1;

Of course, turning that into a loop is decidedly trivial.

I did the best I could creating this program. I have not worked with
this syntax and I am not sure how it works. >> is that a bit shift, I
have see them before but never really used them. I don't get the
reference & to l?. What I am also trying to do is have mutiple
graphics sheets so the data might look like ff 00 ff 00 ff 00 ff

FF00
FF00
FF00
Is how it will be displayed basically I use flat arrays and let the
accessor functions sort out the rest.

So my problem is the +/- bit. I will see if I can make it all
unsigned.

table[0+(counter*8)] = bits[0];
table[1+(counter*8)] = bits[1];
table[2+(counter*8)] = bits[2];
table[3+(counter*8)] = bits[3];
table[4+(counter*8)] = bits[4];
table[5+(counter*8)] = bits[5];
table[6+(counter*8)] = bits[6];
table[7+(counter*8)] = bits[7];

You could also combine the two steps, moving data directly from temp to
table, without using bits[] at all.

for(int l = 0; l != 7; l++){bits[l]=0;}

I'd suggest that you never use 'l' as a variable name again. In many
fonts, it's quite easy to mistake for a '1', hurting readability a great
deal.

--
Later,
Jerry.

The universe is a figment of its own imagination.

OK, I generally use lp. This was so short, I didn't think it would
matter.