袁峰

www.fengyuan.com
随笔 - 24, 评论 - 281, 引用 - 62

导航

标签

每月存档

最新留言

  • 回复: 换工作了
    # 回复: 换工作了 2007-12-18 14:08 vb.net <br>厉害,佩服,学习 <br> <br># 回复: 换工作了 2007-12-18 1...
    by 1(匿名) on 2007/12/29 22:26:00
  • 回复: 换工作了
    ParallelFX !
    by DLU(匿名) on 2007/12/21 15:27:00
  • 回复: 换工作了
    只要不是去google就好
    by fwaef(匿名) on 2007/12/20 15:34:00
  • 回复: 换工作了
    aaaaa
    by s(匿名) on 2007/12/20 12:15:00
  • 回复: 换工作了
    是不是传说中的PLINQ呀?MS中的几个大牛正在做!
    by Boler Guo(匿名) on 2007/12/18 16:50:00
  • 回复: 换工作了
    11年, 佩服! <br>严老大不妨跟大家讲讲具体要做那些工作, 难道是VS上的多核程序调试工具? <br>:)
    by VincentChen(匿名) on 2007/12/18 16:48:00
  • 回复: 换工作了
    看看: <br> <br><a target="_new" href="http://msdn2.microsoft.com/en-us/c...
    by fyuan(匿名) on 2007/12/18 15:06:00
  • 回复: 换工作了
    具体的说说,大家还是很关注老大的
    by 小气的鬼(匿名) on 2007/12/18 14:32:00
  • 回复: 换工作了
    11年,大牛了
    by Victor.Woo(匿名) on 2007/12/18 14:16:00
  • 回复: 换工作了
    Visual Studio / 多核 <br> <br>这两个东西好像交集很小呀。 <br> <br>利用多核的技术提高 Visual Studio ...
    by ghj1976(匿名) on 2007/12/18 14:14:00
  • 回复: 换工作了
    袁老大真是厉害,让我佩服,向你学习!
    by vb.net(匿名) on 2007/12/18 14:09:00
  • 回复: 换工作了
    厉害,佩服,学习
    by vb.net(匿名) on 2007/12/18 14:08:00
  • 回复: 悬赏: large/huge EMF spool file
    sdsdadasdfafvfafdfadsfadsfdfdfgf
    by fsdfsd(匿名) on 2007/12/16 8:52:00
  • 回复: 梁肇新 《编程高手箴言》 书评
    我觉得我这种算是比较肤浅的人都能理解的到的书,你们这些所谓的高手确理解不了而是在讽刺,请问你们的成就是啥,也写出来让我膜拜一下吧。不要说人家做出来的东西是垃圾,在垃圾也有能学习的地方,何况这本书讲了许...
    by lr2000(匿名) on 2007/12/12 20:38:00
  • 回复: 梁肇新 《编程高手箴言》 书评
    不巧变成乱码~ <br> <br>我觉得两方都有各自的道理,但是呢,你说梁只是一个工人,一个垒代码的,确实,一个熟练工完全可以迅速垒出层叠嵌套的代码,再熟练点的,可以找出一个系...
    by lr2000(匿名) on 2007/12/12 20:32:00

广告

 

On August 9th, I posted the following problem on community.csdn.net, under VC/MFC 基础类:

Problem: Write code to generate a bitmap rotated 90 degrees clockwise from the original bitmap, without using any graphics library.

If you want to post your code here, write on your own, do not refer to anything else, no copy/paste, write your code here, review your code before submitting, and include how much time you spend. Add comments in English when possible.

I want to see who can give a good enough answer.

There have been lots of responses to this simple programming problem and some quite interesting discussions. Here is the link to the CSDN thread: http://community.csdn.net/Expert/topic/3254/3254627.xml?temp=.7896845

So here I'm going to comment on those responses.

hahu(地痞 -- 勿近) ( 两星(中级))

#define WIDTHBYTES(bits) (((bits) + 31) / 32 * 4)
=>Global alloc bytes for destination bitmap....
long ldByteWidth = WIDTHBYTES(width*3);     
long ldByteHeight = WIDTHBYTES(height*3);
for(j = 0; j < height; j++)
{
   for(i = 0; i < width ;i++)
   {
       src = j*ldByteWidth + i*3;
       dst = i*ldByteHeight + j*3;
       memcpy(dst, src, sizeof(unsigned char)*3);
   }
}
Is it right? The byte count in this bitmap is ldByteWidth*height

This captures the essence of the problem quite well. Such quick thinking is essential in interviews. But the 'code' as it stands is just a simple sketch, far away from useable code.

ehom(?!) ( 四星(高级))

//位图旋转,24位/32位
int rotated90(const char *filename)
{
 BITMAPFILEHEADER bmpfileheader;
 BITMAPINFOHEADER bmpinfoheader;
 fstream bmpfile;
 bmpfile.open(filename, ios::in|ios::out|ios::binary);
 if(!bmpfile) {
  return -1;
 }
 else {
  bmpfile.read((char *)&bmpfileheader, sizeof(bmpfileheader));
  if(bmpfileheader.bfType!=0x4D42) return -1;
  bmpfile.read((char *)&bmpinfoheader, sizeof(bmpinfoheader));
  if(bmpinfoheader.biBitCount!=24 && bmpinfoheader.biBitCount!=32) return -1;
 }
 //get depth
 int bit = bmpinfoheader.biBitCount/8;     
 //get width of source bitmap
 int bmpWidth = bmpinfoheader.biWidth;
 //get height of source bitmap
 int bmpHeight = bmpinfoheader.biHeight;
 //set height of dest bitmap
 bmpinfoheader.biHeight = bmpWidth;       
 //set width of dest bitmap
 bmpinfoheader.biWidth = bmpHeight;
 //set image size of dest bitmap
 bmpinfoheader.biSizeImage = ((bmpHeight*bit-1)/4+1)*4*bmpWidth;
 //堆上开辟缓存区储存输出位图数据
 char *buf = new char[bmpinfoheader.biSizeImage];
 //定义输出位图各行首地址数组
 unsigned long *scanline = new unsigned long[bmpWidth];
 //get linesize of dest bitmap
 int linesize=bmpinfoheader.biSizeImage/bmpWidth;
 unsigned long n=(unsigned long)&buf[0];        
 //获取输出位图各行在缓存区的首地址
 for(int i=bmpWidth-1;i>=0;i--) {
  scanline[i]=n;
  n+=linesize;                       
 }
 int m=(bmpWidth*bit)%4;
 if(m==0) {       
  for(int i=0;i   for(int j=0;j    bmpfile.read((char *)(scanline[j]+i*bit), bit);
   }               
  }
 }
 else {
  m=4-m;
  char *tempbuf=new char[m];
  for(int i=0;i   for(int j=0;j    bmpfile.read((char *)(scanline[j]+i*bit), bit);
   }
   bmpfile.read(tempbuf, m);
  }
 }
 //write head and image data
 bmpfile.seekg(0);
 bmpfileheader.bfSize=bmpinfoheader.biSizeImage+sizeof(bmpfileheader)+sizeof(bmpinfoheader);
 bmpfile.write((char *)&bmpfileheader, sizeof(bmpfileheader));
 bmpfile.write((char *)&bmpinfoheader, sizeof(bmpinfoheader));
 bmpfile.write(buf, bmpinfoheader.biSizeImage);     
 bmpfile.close();
 delete[] buf;
 return 0;
}
time 10-20 mins

Apparently the author is an experienced programmer. But the code here solves a slightly different problem, it loads a bitmap from disk and writes back a rotated image. In interviews, you have to focus on the problem given to give the best answer, so you can't afford to work on anything else (like file I/O here). Also, what is required (implicitly) is to write reusable modular code, not a demo program.

yuliangpei(踏雪无痕) ( 二级(初级))

int i,j;
for(i = 0; i < Width; i++)
{
    for(j = 0; j    {
       pDst[j][Width-i] = pSrc[i][j];
   }
}
其中,数据复制这一段不能直接用于程序,应该根据位图的bitCount而进行调整。程序是对应象素来写的。信手写的,不知对否?

Good understanding of the problem, far from complete usable code. Using two dimensional array notation is good for understanding the problem, but if you use that in your real code, it's less general or less efficient. One small mistake, Width - 1 - i should be used in place of Width -i.

ehom(?!) ( 四星(高级))

#define getSize(width, height, depth) ((width*depth*3)/4)*4*height

 

struct BitmapData

{

int width;

int height;

void *pScan0; // pointer to first scanline

int bpp; // bits per pixel

int stride; // distance between scanlines

};

 

int rotated90(BitmapData bmpdata)

{

if (bmpdata.bpp < 8) return -1;
 //bytes of depth
 int depth=bmpdata.bpp/8;     
 //width of source bitmap
 int bmpWidth=bmpdata.width;
 //height of source bitmap
 int bmpHeight=bmpdata.height;
 //image size of dest bitmap
 int imagesize=getSize(bmpHeight, bmpWidth, depth);
 //堆上开辟缓存区储存输出位图数据
 char *bufdst=new char[imagesize];
 //数组储存输出位图各行首地址
 unsigned long *scanlinedst=new unsigned long[bmpWidth];
 //输出位图每行数据长度 
 int linesize=imagesize/bmpWidth;
 int n=(unsigned long)bufdst;        
 //获取输出位图各行在缓存区的首地址
 for(int i=bmpWidth-1;i>=0;i--) {
  scanlinedst[i]=n;
  n+=linesize;                       
 }
 
 char *buf=(char *)bmpdata.pScan0;
 int m=(bmpWidth*depth)%4;
 if(m!=0)m=4-m;
 for(int i=0;i  for(int j=0;j   memcpy((char *)(scanlinedst[j]+i*depth), buf, depth);
   buf+=depth;
  }
  buf+=m;
 }
 memcpy(bmpdata.pScan0, bufdst, imagesize);
 delete[] scanlinedst;
 delete[] bufdst; 
 return 0;

}

 

int openfile(const char *filename) // omitted

To ehom's second version, I said “Quite a few problems in ehon's code”. Here are they:

  1. The function prototype int rotated90(BitmapData bmpdata) is wrong. The original problem askes you to generate a (new) rotated image from an existing image, there is no way a new image can be returned. bmpdata is passed as value type, it can't even modify an existing image to its rotated image. Rotate90 will be a better name.
  2. bmpdata.bpp/8 does not handle bpp=15.
  3. getSize, needs to check for multiplication overflow
  4. No allocation failure check for new char[imagesize]
  5. No allocation failure check for new unsigned long[bmpWidth]. This is also not needed. Also bmpWidth * sizeof(unsigned long) could overflow integer range.
  6. memcpy inside nested loop is slow. Do not use that in performance critical code.
  7. memcpy(bmpdata.pScan0, bufdst, imagesize) is dangerous, rotated image could be larger than original image because of scanline alignment.
  8. scanlinedst and n, mixing pointers with integer will generate compile time error/warning. It does not work on 64-bit compiler, because int is still 32-bit while pointers are 64-bit.

holyeagle(一杯清茶) ( 一星(中级))

#define GetImageSize(width,height,nCount) ((width*nCount+31)/32)*4*height 
#define OPENBMP(n) "d:\\bmp\\"#n".bmp"
#define SAVEBMP(n) "d:\\bmp\\"#n"R.bmp"


typedef struct tagRotateINfo
{
 INT  nWidth;
 INT  nHeight;
 VOID *pScan0; // pointer to first scanline
 INT  nBpp;  // bits per pixel
 INT  nSrcStride; // distance between scanlines of Src
 INT  nDstStride; // distance between scanlines of dst
 VOID *pDst;  // pointer to destination 
}ROTATEINFO, *LPROTATEINFO;

 

BOOL CRotateImageDoc::Rotateclockwise90()
{
 // read image file
 CFile file;
 
 if (!file.Open(OPENBMP(1), CFile::modeRead) )
 {
  return FALSE;
 }
 
 // get file header
 BITMAPFILEHEADER FileHeader ={0};

 file.Read(&FileHeader, sizeof(BITMAPFILEHEADER));
 
 if (FileHeader.bfType != ((WORD) ('M' << 8) | 'B'))
 {
  return FALSE;
 }
 
 // get bitmap info
 DWORD dwBmpInfo = FileHeader.bfOffBits - sizeof(BITMAPFILEHEADER);
 DWORD dwRgb = dwBmpInfo - sizeof(BITMAPINFOHEADER);
 
 //apply memory
 LPBITMAPINFO pBmpInfo = (LPBITMAPINFO)new BYTE[dwBmpInfo];

 if (pBmpInfo == NULL)
 {
  file.Close();
  return FALSE;
 }
 memset(pBmpInfo, 0, sizeof(BYTE)*dwBmpInfo);
 
 file.Read(pBmpInfo, dwBmpInfo);
 if (pBmpInfo->bmiHeader.biBitCount >= 8 && pBmpInfo->bmiHeader.biCompression!=0)
 {
  file.Close();
  return FALSE;
 }
 
 // Get image data line by line
 DWORD dwWidth = pBmpInfo->bmiHeader.biWidth;
 DWORD dwHeight = pBmpInfo->bmiHeader.biHeight;
 WORD wBitCount = pBmpInfo->bmiHeader.biBitCount;
 
 DWORD dwSrcSize = GetImageSize(dwWidth, dwHeight, wBitCount);
 DWORD dwDstSize = GetImageSize(dwHeight, dwWidth, wBitCount);
 
 LPBYTE pSrcData = new BYTE[dwSrcSize];
 LPBYTE pDstData = new BYTE[dwDstSize];

 ASSERT(NULL != pSrcData && NULL != pDstData);

 ZeroMemory(pSrcData, sizeof(BYTE)*dwSrcSize);
 ZeroMemory(pDstData, sizeof(BYTE)*dwDstSize);
 
 DWORD dwColorTable = dwRgb/sizeof(RGBQUAD);
 RGBQUAD *pRgbQuad = new RGBQUAD[dwColorTable];

 ASSERT(NULL != pRgbQuad);
 ZeroMemory(pRgbQuad, dwRgb);
 
 // read
 LPBYTE lpTemp = pSrcData;
 INT nSrcStride = ((dwWidth*wBitCount+31)/32)*4;
 INT nDstStride = ((dwHeight*wBitCount+31)/32)*4;
 
 for (DWORD i = 0; i < dwHeight; ++i)
 {
  file.Read(lpTemp, nSrcStride);
  lpTemp += nSrcStride;
 }

 file.Seek(sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER), CFile::begin);
 file.Read(pRgbQuad, dwRgb);
 
 file.Close();
 

 // set rotate infomation
 ROTATEINFO RoInfo = {0};

 RoInfo.nWidth = pBmpInfo->bmiHeader.biWidth;
 RoInfo.nHeight = pBmpInfo->bmiHeader.biHeight;
 RoInfo.pScan0 = pSrcData;
 RoInfo.pDst = pDstData;
 RoInfo.nBpp = pBmpInfo->bmiHeader.biBitCount;
 RoInfo.nSrcStride = nSrcStride;
 RoInfo.nDstStride = nDstStride;

 RotateImage(RoInfo);

 // store file
 CFile fileW;

 fileW.Open(SAVEBMP(1), CFile::modeCreate|CFile::modeReadWrite);

 // write head info
 fileW.Write(&FileHeader, sizeof(BITMAPFILEHEADER));

 // write bitmap info
 pBmpInfo->bmiHeader.biWidth = RoInfo.nHeight;
 pBmpInfo->bmiHeader.biHeight = RoInfo.nWidth;

 fileW.Write(pBmpInfo, dwBmpInfo);

 // write file data line by line
 lpTemp = (LPBYTE)RoInfo.pDst;
 for (i = 0; i < dwWidth; ++i)
 {
  fileW.Write(lpTemp, nDstStride);

  lpTemp += nDstStride;
 }

 fileW.Close();

 return TRUE;
}

BOOL CRotateImageDoc::RotateImage(ROTATEINFO RoInfo)
{
//////////////////////////////////////////////////////////////////////////
/* I just support above 8 bits image,
 the bits of 1,2,4, I will convert to 24 bits at first, then do them as normal*/
//////////////////////////////////////////////////////////////////////////
 
 // get src prototype with color bits
 // if bpp is 8 bits, the step is one byte; if bpp is 16 bits, the step is two bytes, as soon on.

 INT nStep = RoInfo.nBpp / 8;
 INT nSrcStride = RoInfo.nSrcStride;
 INT nDstStride = RoInfo.nDstStride;
 INT nDiff = nSrcStride - RoInfo.nWidth*nStep;

 // check the src and dst are validate both.
 ASSERT(NULL != RoInfo.pScan0 && NULL != RoInfo.pDst);
 
 LPBYTE pSrcLine = (LPBYTE)RoInfo.pScan0, pDstLine = (LPBYTE)RoInfo.pDst;  //pointer to working line of Src & Dst buffer
 LPBYTE pSrcPixel = NULL, pDstPixel = NULL; //pointer to working pixel of Src & Dst buffer

 for (INT i = 0; i < RoInfo.nHeight; ++i)
 {
  pSrcPixel = pSrcLine + (nSrcStride - nStep - nDiff);
  pDstPixel = pDstLine;
  
  for (INT j = 0; j < RoInfo.nWidth; ++j)
  {
   memcpy(pDstPixel, pSrcPixel, nStep);

   pDstPixel += nDstStride;
   pSrcPixel -= nStep;
  }

  //get next line
  pSrcLine += nSrcStride;
  pDstLine += nStep;
 }
 
 return TRUE;
}

 

It taken me about three hours, too long!
Hope I can do it better next time.

Mostly similar problems, plus some new:

  1. File IO is not required.
  2. ROTATEINFO is not a good design, it mixes source bitmap with destination bitmap.
  3. CFile, avoid using MFC when possible.
  4. OPENBMP(1), write reusable code, not sample code, pass the file name as parameter
  5. pBmpInfo and other memory allocation could be leaked.
  6. GetImageSize could overflow.
  7. ASSERT(NULL ! pSrcData && NULL != pDstData) is not valid, allocation could fail.
  8. Bitmap could still be compressed.
  9. RotateImage need runtime check for 1,2,4 bpp and when nBpp is not a multiple of 8.
  10. memcpy is slow for performance critical code.

 joysunstar(鹤鸣) ( 二级(初级))

void Rotatebmp90() 
{
// this program rotate a 24bits-bitmap file by 90 degrees 
CFileDialog fileDlg(TRUE ,NULL,NULL ,NULL,"bitmap file(*.bmp)|*.bmp||") ;
if(fileDlg.DoModal() != IDOK )
 return ;
CString strFileName = fileDlg.GetPathName() ;
CFile bmpFile(strFileName,CFile::modeRead) ;
BITMAPFILEHEADER bmfh ;
bmpFile.Read(&bmfh,sizeof(BITMAPFILEHEADER) ) ;
if(bmfh.bfType != 0x4d42 )
 return ;
BITMAPINFOHEADER bmih ;
bmpFile.Read(&bmih,sizeof(BITMAPINFOHEADER)) ;
if(bmih.biBitCount != 24)
{
     AfxMessageBox("not a true color image !",MB_ICONINFORMATION) ;
     return ;
}
     
bminfo->bmiHeader = bmih ;
bmNewInfo->bmiHeader = bmih ;
DWORD width = bmih.biWidth ;
DWORD height = bmih.biHeight ;
DWORD offwidth = (width + 3) & ~3 ;
DWORD offHeight = (height + 3) & ~3 ;
bmNewInfo->bmiHeader.biHeight = width ;
bmNewInfo->bmiHeader.biWidth = height ;
bmNewInfo->bmiHeader.biSizeImage = offHeight * width * 3 ;
 
m_ImgWidth = width ;
m_ImgHeight = height ;
if(lpSrc)
{
free(lpSrc) ;
lpSrc = NULL ;
}
if(lpDest)
{
free(lpDest) ;
lpDest = NULL ;
}
 lpSrc = (LPBYTE)malloc(bmih.biSizeImage) ;
 lpDest = (LPBYTE)malloc(width * offHeight * 3) ;
 ZeroMemory(lpDest,width * offHeight * 3) ;
 bmpFile.Read(lpSrc,bmih.biSizeImage) ;
 BYTE *pixel ;
 
 for(DWORD i = 0 ; i < height ;i++)
 {
 for(DWORD j = 0 ;j < width ;j++)
 {
 pixel = lpSrc + i * offwidth * 3 + j * 3;     
 *(lpDest + j * offHeight * 3 + (height - i) * 3) = *pixel;
 *(lpDest + j * offHeight * 3 + (height - i) * 3 + 1) = *(pixel + 1 ) ;
 *(lpDest + j * offHeight * 3 + (height - i) * 3 + 2) = *(pixel + 2 );
 }
 }
 bmpFile.Close() ;
 UpdateAllViews(NULL) ;
}

  1. File selection dialog box and file I/O is not required.
  2. Where is bminfo and bmNewInfo declared?
  3. offHeight * 3 is not the same as (height * 3 + 3 ) /4 * 4
  4. biSizeImage could overflow.
  5. Where is lpSrc and lpDest declared? Member variable of the class?
  6. The code within the nested loop is really slow.

ALNG(?) ( 一级(初级))

// start at 9:30
// Define a struct or class to store information about a bitmap:
struct BitmapInfo
{
public:
 // Ctors, Detors, skipped
 /// @brief rotate @c *this 90 degree clockwise.
 ///
 void rotate90Clockwise();
private:
 int    width;
 int    height;
 void * pScan0;   // pointer to first scanline
 int    bpp;      // bits per pixel
 int    stride;   // distance between scanlines
private:
 /// @brief: internally used to get byte width of a scanline
 inline unsigned lineWidth()const{ return stride;  }
 
 /// @brief: offest of specified pixel from base address
 inline unsigned pixelOffest(unsigned row, unsigned col)const{
  return  row*lineWidth() + col*bbp;
 }
 /// @brief: internally used helper function to get the address
 /// of the pixel at row and col
 /// @param row row of the specified pixel, count from 0
 /// @param col col of the specified pixel, count from 0.
 /// @return address of the specified pixel
 inline char * pixel(unsigned row, unsigned col)const{
  return reinterpret_cast(pScan0)+ pixelOffset(row,col);
 }
 
 /// @brief: internally used to copy a pixel from @c from to @c to
 inline void copyPixel(const char * from, char * to)const{
  for(unsigned i=0; i   to[i]=from[i];
 }
};
// 11 12 13 14 
// 21 22 23 24
// 31 32 33 34
// ==>
// 31 21 11
// 32 22 12
// 33 23 13
// 34 24 14
//
// should be possible to work out a O(1) space complexity algorithm
// here I use the plain O(widht*height) one
//
// a quick but less smart solution
//
void BitmapInfo::rotate90Clockwise()
{
 BitmapInfo tmp(*this); // make a copy 
 
 height = width;
 width = tmp.height;
 // set stride properly.
 
 // do the actual rotation
 for(unsigne r=0; r  for(unsigned c=0; c   copyPixel( tmp.pixel(r,c), pixel(c,width-r-1);
}

  1. Should BitmapInfo be a class, instead of a struct?
  2. Quite a few mixing of unsigned with int.
  3. Critical BitmapInfo copy constructor skipped.
  4. Rotated image may be larger than original image in space because of scanline alignment. Simply switching height and width does not give you rotated image, stride may be wrong after that.
  5. copyPixel copies source pixels into tmp bitmap, which is then thrown away?
  6. The rotating loop is slow, you're trusting compiler to be able to do all the optimizations.
  7. Not a single error check.

ambochan(打杂) ( 五级(中级))

Lots of NT GDI calls

Does not meet requirement. Do not use any graphics library.

Your job could be to implement those graphics library, not using it as an application programmer.

 huanyun(无妻徒刑) ( 两星(中级))

typedef struct tagXIMGDATA
{
 int    width;
 int    height;
 void * pScan0;   // pointer to first scanline
 int    bpp;      // bits per pixel
 int    stride;   // distance between scanlines
} XIMGDATA;
void FreeImg(XIMGDATA& ImgDst)
{
 if(ImgDst.pScan0 != NULL)
 {
  delete [] ImgDst.pScan0;
  ImgDst.pScan0 = NULL;
  ImgDst.width  = 0;
  ImgDst.height = 0;
  ImgDst.bpp    = 0;
  ImgDst.stride = 0;
 }
}
int RotateImg(XIMGDATA& ImgDst,const XIMGDATA& ImgSrc)
{
 int nRet = -1;
 FreeImg(ImgDst);
 int nWidthS  = ImgSrc.width;
 int nHeightS = ImgSrc.height;
 int nWidthD  = nHeightS;
 int nHeightD = nWidthS;
 int nBitCount = ImgSrc.bpp;
 int nRowLenSrc = ImgSrc.stride;
 BYTE* pSrc = static_cast(ImgSrc.pScan0);
 if (nWidthS < 1 || nHeightS < 1
  || nWidthS > 10000 || nHeightS > 10000
  || nBitCount < 1 || nBitCount > 64
  || nRowLenSrc < 0 ||  pSrc == NULL)
  return nRet;
 int nRowLenDst = nWidthD * nBitCount / 8;
 if (nBitCount == 1 && nWidthD % 8) ++nRowLenDst;
 nRowLenDst =  ((nRowLenDst + 3) & ~3);
 
 BYTE *pDst = new BYTE[nHeightD*nRowLenDst];
 if(pDst == NULL)
 {
  FreeImg(ImgDst);
  return nRet;
 }
 memset(pDst, 0, nHeightD*nRowLenDst);
 ImgDst.pScan0 = static_cast(pDst);
 ImgDst.width  = nWidthD;
 ImgDst.height = nHeightD;
 ImgDst.bpp    = nBitCount;
 ImgDst.stride = nRowLenDst;
 int i,x,y,nPosS=0,nPosD=0;
 int nPixelByte = ImgDst.bpp>>3;
 BYTE bMaskD,bMaskS;
 switch(nBitCount)
 {
 case 1:
  for(y=0;y  {
   nPosD =  nRowLenDst*y;
   nPosS =  y>>3;
   bMaskD = static_cast( 0x80 );
   bMaskS = static_cast( 0x80 >> (y % 3) );
   x = 0;
   while(x < nWidthD)
   {
    if(pSrc[nPosS] & bMaskS)
    {
     pDst[nPosD] |= bMaskD;
    }
    ++x;
    bMaskD >>=1;
    nPosS  += nRowLenSrc;
    if(bMaskD == 0)
    {
     bMaskD=0x80;
     ++nPosD;
    }
   }
   for(x=0;;++x)
   {
    pDst[nPosD] = pSrc[nPosS];
    nPosD += nPixelByte;
    nPosS += nRowLenSrc;
   }
  }
  nRet = 0;
  break;
 case 8:
 case 16:
 case 24:
 case 32:
  for(y=0;y  {
   nPosD = nRowLenDst*y;
   nPosS = y*nPixelByte;
   for(x=0;x   {
    for(i=0;i    {
     pDst[nPosD+i] = pSrc[nPosS+i];
    }
    nPosD += nPixelByte;
    nPosS += nRowLenSrc;
   }
  }
  nRet = 0;
  break;
 default:
  break;
 }
 return nRet;
}

  1. Why not use a class? Then FreeImg and RotateImg will just be member functions.
  2. Missing important constructor, at least to set pScan0 to NULL.
  3. delete [] ImgDst.pScan0, pScan0 is void pointer.
  4. pSrc can be const pointer.
  5. Checking for nWidthS, nHeightS larger than 10000 is good, but too restrictive.
  6. Checking for nBitCount > 64 is too restrictive, GDI+ supports ARGB 16-bit per channel images
  7. nRowLenSrc could be negative for bottom-up images.
  8. Why ++nRowLenDst?
  9. Check for nHeightD * nRowLenDst overflow
  10. For each pixel writing to destination using pDst[nPosD] |= bMaskD is slow.
  11. Why for (x=0;;++x)? When does it end?
  12. Slow rotating loop.

tuqvb(风间苍月) ( 一级(初级))

// ***********************************************************
// ohoh, only a kernel function
// time: 30 min
// ***********************************************************
// PixelS_Type  : type of source image pixel
// PixelD_Type  : type of dest image pixel
// wid          : width of source image and height of dest image, in pixel
// hei          : height of source image and width of dest image, in pixel
// psrc         : address of first pixel of source image
// lPitchSrc    : the length of source scan line, in bytes
// pdst         : address of first pixel of dest image
// lPitchDst    : the length of dest scan line, in bytes
// note         : there must be a convert from source to dest
template
void Ration90(int wid, int hei,
  const PixelS_Type *psrc, int lPitchSrc,
  PixelD_Type *pdst, int lPitchDst)
{
 char *plsrc= reinterpret_cast(const_cast(psrc));  // scanline address of source
 int xdoffset= (hei - 1) * sizeof(PixelD_Type);     // pixel address offset of dest
 for (int y= 0; y < hei; ++y)
 {
  char *pldst= reinterpret_cast(pdst);    // scanline address of dest
  int xsoffset= 0;       // pixel address offset of source
  for (int x= 0; x < wid; ++x)
  {
   // movx dst[x][hei - 1 - y], src[y][x]
   *reinterpret_cast(pldst + xdoffset)=
    *reinterpret_cast(plsrc + xsoffset);
   pldst+= lPitchDst;
   xsoffset+= sizeof(PixelS_Type);
  }
  plsrc+= lPitchSrc;
  xdoffset-= sizeof(PixelD_Type);
 }
}
// ***********************************************************
// use sample
typedef short PIXEL555;
typedef unsigned short PIXEL565;
typedef int PIXEL32;
typedef struct _stPixel24
{
 char r;
 char g;
 char b;
 _stPixel24& operator= (PIXEL32 p32){ return *this;}
 _stPixel24& operator= (PIXEL555 p16){ return *this;}
 _stPixel24& operator= (PIXEL565 p16){ return *this;}
 operator PIXEL32(void){ return 0;}
 operator PIXEL555(void){ return 0;}
 operator PIXEL565(void){ return 0;}
}PIXEL24;
#define WIDTH 83
#define HEIGHT 85
PIXEL24 image24[WIDTH][HEIGHT];
PIXEL32 image32[HEIGHT][WIDTH];
PIXEL565 image565[HEIGHT][WIDTH];
PIXEL555 image555[HEIGHT][WIDTH];
Ration90(WIDTH, HEIGHT, &image32[0][0], WIDTH * sizeof(PIXEL32), &image24[0][0], HEIGHT * sizeof(PIXEL24));
Ration90(WIDTH, HEIGHT, &image565[0][0], WIDTH * sizeof(PIXEL565), &image24[0][0], HEIGHT * sizeof(PIXEL24));
Ration90(WIDTH, HEIGHT, &image555[0][0], WIDTH * sizeof(PIXEL555), &image24[0][0], HEIGHT * sizeof(PIXEL24));
Ration90(HEIGHT, WIDTH, &image24[0][0], WIDTH * sizeof(PIXEL24), &image32[0][0], HEIGHT * sizeof(PIXEL32));
Ration90(HEIGHT, WIDTH, &image24[0][0], WIDTH * sizeof(PIXEL24), &image565[0][0], HEIGHT * sizeof(PIXEL565));
Ration90(HEIGHT, WIDTH, &image24[0][0], WIDTH * sizeof(PIXEL24), &image555[0][0], HEIGHT * sizeof(PIXEL555));

Finally, someone thought about using template!! I acutally used template in production code (part of Avalon code). But you should at least explain why did you choose to use template. The benefit of using template is to avoid memcpy in the inner loop. The binary code generated will be larger, but it will be much faster, because each instantiation will be for a constant color depth. Compiler will be able to generate the minimum code for that case.

  1. pldst and xdoffset can be merged into a single variable.
  2. plsrc and xoffset can be merged into a single variable.
  3. Code is not complete. The problem is to write (generic) code to generate a rotated image from an image.

 

To be fair, here is my code

FengYuanMSFT

class Image

{

public:

int m_width;

int m_height;

int m_bpp;

int m_stride;

void * m_pScan0;

 

Image()

{

m_pScan0 = null;

}

 

~Image()

{

if (m_pScan0)

{

delete [] (char *) m_pScan0;

m_pScan0 = null;

}

}

 

Image * Rotate90() const;

};

 

template<typename Pixel>

Rotate(Image * pDst, const Image * pSrc)

{

for (int y = 0; y < pDst->m_height; y ++)

{

const Pixel * pS = (Pixel *) ((char *) pSrc->m_pScan0 + (pSrc->m_Height - 1) * pSrc->Stride) + y;

Pixel * pD = (Pixel *) ((char *) pDst->m_pScan0 + y * pDst->stride);

 

for (int x = 0; x < pDst->m_width; x ++)

{

* pD ++ = * pS;

pS = (Pixel *) ((char *) pS - pSrc->m_Stride);

}

}

}

 

#pragma pack(push, 1)

typedef struct { char one; char two; char three; } triple;

#pragma pack(pop)

 

Image * Image::Rotate90() const

{

// Parameter validation

if (m_width < 0 || (m_height < 0) || (m_bpp < 1))

{

return null;

}

 

if (((MAX_INT-31) / m_bpp) < m_height) // avoid overflow

{

return null;

}

 

int stride = (m_height * m_bpp + 31) / 32 * 4; // dword align

if ((MAX_INT / stride) < m_width) // avoid overflow

{

return null;

}

 

Image * pResult = new Image();

 

if (pResult == null)

{

return null;

}

 

pResult->m_width = m_height;

pResult->m_height = m_width;

pResult->m_bpp = m_bpp;

pResult->m_stride = stride;

pResult->m_Scan0 = new char[stride * m_width];

 

if (pResult->m_Scan0 == null)

{

delete pResult;

return null;

}

 

switch (m_bpp)

{

case 8:

Rotate<char>(pResult, this); break;

 

case 15:

case 16:

Rotate<short>(pResult, this); break;

 

case 24:

Rotate<triple>(pResult, this); break;

 

case 32:

Rotate<long>(pResult, this); break;

 

case 64:

Rotate<double>(pResult, this); break;

 

default:

assert(false); // new image format

delete pResult;

pResult = null;

}

 

return pResult;

}

Some explanations:

  1. It's OK to write a simple solution using memcpy or a loop to copy each pixels. But you need to understand that such a solution is considered slow for a high performance graphics library. For 1000 x 1000 pixel image, the inner loop will be executed 1 million times.
  2. memcpy has two implementations: one inline implementation and one out-of-line function implementation. The inline version should be faster for small data size, the out-of-line implementation is optimized for copying large amount of data.
  3. memcpy is considered slow here because the size parameter is a variable, so compiler/runtime library has to use a loop to implement it. Any loop or conditional jump is considered slow in such an inner loop.
  4. Using template is faster here because each instance of the template knows its pixel size as a constant, so copying a pixel can be implemented in a single instruction for 8-bpp 15-bpp, 16-bpp, 32-bpp images, much faster than a loop.
  5. Checking for multiplication overflow is essential to avoid heap buffer overrun which could result in security risks.
  6. The code here is not compiled, nor tested.

Thanks for all the replies.

打印 | 张贴于 2004-08-18 02:27:00 | Tag:暂无标签

留言反馈

#re: Rotate a bitmap by 90 degrees 编辑
if
BitCount%8==0;
we could to transfort pixels data in loop in memory;1Pixel once;
but when BitCount%8!=0,CPU will be much used

Now I see GDI is good.
2007-11-10 12:12:00 | [匿名:Kuikui]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
// Parameter validation if (m_width < 0 || (m_height < 0) || (m_bpp < 1)) { return null; }

should be

// Parameter validation if (m_width < 1 || (m_height < 1) || (m_bpp < 1)) { return null; }
2005-07-13 01:13:00 | [匿名:printing]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
/*assume a pixel is one int, like R,G,B,A */
//I guest the point to avoid mutliplication and use simple add and stubstraction
int* Rotate(int* data, int width, int length)
{
int* result = (int *)malloc(width*length*sizeof(int));
if (0 == result) return result;

//
int resultStart = length-1;
int sourceStart = 0;
for (int j = 0; j< length; j++)
{
y = resultStart;
for (int i=0; i< width; i++)
{
result[y] = data[sourceStart++];
y += length;
}
resultStart--;
}

return result;
}
2005-07-12 07:50:00 | [匿名:printing]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
测试代码(win32 helloworld project.):
// ...(略)
case ID_SAVE: // 保存
g_pdib->Save("abc.bmp");
break;
case ID_ROTATE: // 旋转
pDib = g_pdib->Rotate90();
delete g_pdib;
g_pdib = pDib;
GetClientRect(hWnd, &rect);
InvalidateRect(hWnd, &rect, FALSE);
break;
// ...(略)
case WM_PAINT: // 显示
hdc = BeginPaint(hWnd, &ps);
// TODO: Add any drawing code here...
RECT rt;
GetClientRect(hWnd, &rt);
// DrawText(hdc, szHello, strlen(szHello), &rt, DT_CENTER);
pt.x = pt.y = 0;
g_pdib->Show(hdc, pt);
EndPaint(hWnd, &ps);
break;


----- 问题如下 ---
当代码执行到:
for(i = 0; i < desHeight; ++i) {
const pixel* pS = (pixel*)((BYTE*)pSrcData + srcBpl * srcHeightDec) + i ;
pixel* pD = (pixel*)((BYTE*)pDesData + desBpl * (desHeightDec - i));
for(j = 0; j < desWidth; ++j) {
*pD++ = *pS;
pS = (pixel*)((BYTE*)pS - srcBpl);
}
}
当 i = 888 时,程序出错,pS指针超出。调试了2个小时,实在没有结果。

谢谢大家的关注。
2004-08-20 18:33:00 | [匿名:enoloo]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
求助: 调试了2个小时,旋转还是有问题。下面是个简单的测试--

// dib.h
#ifndef _DIB_H_
#define _DIB_H_

#include <windows.h>

#pragma pack(push,1)
typedef struct {BYTE a; BYTE b; BYTE c; } triple;
#pragma pack(pop)

class CDib
{
public:
CDib();
~CDib();
bool Load(LPCTSTR strFileName);
bool Save(LPCTSTR strFileName);
void Show(HDC hdc, POINT pt);
// closewise rotate 90 degree. and reset the bitmap file also.
CDib* Rotate90() const;
public:
long m_nBpl; // BYTES per line
long m_szRGBQuad;
BITMAPINFOHEADER m_bmHeader;
RGBQUAD* m_pRGBQuad;
BYTE* m_pData;
};
#endif

// dib.cpp
#include "dib.h"
#include <fstream>
#include <assert.h>

template<typename pixel>
void g_Rotate90(const CDib* pSrc, CDib* pDes)
{
long desWidth = pDes->m_bmHeader.biWidth;
long desHeight = pDes->m_bmHeader.biHeight;
long desHeightDec = pDes->m_bmHeader.biHeight - 1;
long srcHeightDec = pSrc->m_bmHeader.biHeight - 1;
long srcBpl = pSrc->m_nBpl;
long desBpl = pDes->m_nBpl;
BYTE* pSrcData = pSrc->m_pData;
BYTE* pDesData = pDes->m_pData;
int i,j;

for(i = 0; i < desHeight; ++i) {
const pixel* pS = (pixel*)((BYTE*)pSrcData + srcBpl * srcHeightDec) + i ;
pixel* pD = (pixel*)((BYTE*)pDesData + desBpl * (desHeightDec - i));
for(j = 0; j < desWidth; ++j) {
*pD++ = *pS;
pS = (pixel*)((BYTE*)pS - srcBpl);
}
}
}
CDib::CDib()
{
m_pRGBQuad = NULL;
m_pData = NULL;
m_szRGBQuad = 0;
m_nBpl = 0;
}

CDib::~CDib()
{
m_szRGBQuad = 0;
m_nBpl = 0;
if(m_pData != NULL)
{
delete m_pData;
m_pData = NULL;
}
if(m_pRGBQuad != NULL)
{
delete m_pRGBQuad;
m_pRGBQuad = NULL;
}
}

bool CDib::Load(LPCTSTR strFileName)
{
std::ifstream inFile(strFileName,std::ios::binary | std::ios::in);
if(!inFile.is_open())
return false;

// get the file size.
inFile.seekg(0,std::ios::end);
const long szFile = inFile.tellg();
if(szFile <= 0)
{
inFile.close();
return false;
}

WORD bmFlag[7];
inFile.seekg(std::ios::beg);
inFile.read((char*)bmFlag, sizeof(WORD)*7);

if(bmFlag[0] != ((WORD)'B' | 'M'<<8)) // it's not a bitmap file.
{
inFile.close();
return false;
}

inFile.read((char*)&m_bmHeader, sizeof(BITMAPINFOHEADER));
long width = m_bmHeader.biWidth;
long height = m_bmHeader.biHeight;
long bpp = m_bmHeader.biBitCount;
if(bpp < 1)
{
inFile.close();
return false;
}
// BYTES per line = (bits + 31) / 32 * 4.
m_nBpl = ((width * bpp + 31) >> 5) << 2;

long clrUsed = m_bmHeader.biClrUsed;
if(clrUsed == 0)
{
switch(bpp) {
case 1:
m_szRGBQuad = 2; break;
case 4:
m_szRGBQuad = 16;
break;
case 8:
m_szRGBQuad = 256;
break;
case 16:
// NO color table,the case : m_bmHeader.biCompression == BI_RGB
m_szRGBQuad = 0;
break;
default:
m_szRGBQuad = 0;
}
}
else
{
m_szRGBQuad = clrUsed;
}

// match the color table.
if(m_szRGBQuad != 0)
{
m_pRGBQuad = new RGBQUAD[m_szRGBQuad];
inFile.read((char*)m_pRGBQuad, sizeof(RGBQUAD)*m_szRGBQuad);
}
long nData = height * m_nBpl;
m_pData = new BYTE[nData]; // allocate a buffer for bm data.
inFile.read((char*)m_pData, sizeof(BYTE) * nData);

inFile.close();
return true;
}

bool CDib::Save(LPCTSTR strFileName)
{
std::ofstream outFile(strFileName,std::ios::binary | std::ios::out | std::ios::trunc);
if(!outFile.is_open())
return false;

WORD bmFlag[7]; // for BitmapFileHeader with 1 alignment.
bmFlag[0] = (WORD('B') | 'M'<<8);

long nData = m_bmHeader.biHeight * m_nBpl;
DWORD szFile = (sizeof(WORD) * 7) + sizeof(BITMAPINFOHEADER) +
(sizeof(RGBQUAD) * m_szRGBQuad) + (sizeof(BYTE) * nData);

bmFlag[1] = LOWORD(szFile) ;
bmFlag[2] = HIWORD(szFile);

DWORD szOffData = (sizeof(WORD) * 7) + sizeof(BITMAPINFOHEADER) +
(sizeof(RGBQUAD) * m_szRGBQuad);

bmFlag[5] = LOWORD(szOffData);
bmFlag[6] = HIWORD(szOffData);

outFile.write((char*)bmFlag, sizeof(WORD) * 7);

outFile.write((char*)&m_bmHeader, sizeof(BITMAPINFOHEADER));

if(m_szRGBQuad != 0)
outFile.write((char*)m_pRGBQuad, sizeof(RGBQUAD) * m_szRGBQuad);

outFile.write((char*)m_pData, sizeof(BYTE) * nData);

outFile.close();
return true;
}

void CDib::Show(HDC hdc, POINT pt)
{
assert(hdc);
HBITMAP hBmp = CreateDIBitmap(hdc, &m_bmHeader, CBM_INIT, m_pData,
(BITMAPINFO*)&m_bmHeader, DIB_RGB_COLORS);

HDC hMemDC = CreateCompatibleDC(hdc);
HBITMAP hOld = (HBITMAP)SelectObject(hMemDC, hBmp);
BitBlt(hdc, pt.x, pt.y, m_bmHeader.biWidth, m_bmHeader.biHeight,
hMemDC, 0, 0, SRCCOPY);
SelectObject(hMemDC,hOld);
DeleteObject(hBmp);
DeleteDC(hMemDC);

return;
}

CDib* CDib::Rotate90() const
{
// first, check!
int bitCount = m_bmHeader.biBitCount;
if(!m_pData || bitCount % 8)
return NULL;

long newWidth = m_bmHeader.biHeight;
long newHeight = m_bmHeader.biWidth;

// second, create a new instance of CDib.
CDib* pRet = new CDib();
pRet->m_bmHeader = m_bmHeader;
pRet->m_bmHeader.biHeight = newHeight;
pRet->m_bmHeader.biWidth = newWidth;
pRet->m_nBpl = ((newWidth * bitCount + 31) >> 5) << 2;
pRet->m_szRGBQuad = m_szRGBQuad;
if( m_szRGBQuad != 0)
memcpy( pRet->m_pRGBQuad, m_pRGBQuad, sizeof(RGBQUAD) * m_szRGBQuad);

pRet->m_pData = new BYTE[pRet->m_nBpl * newHeight];
if(!pRet->m_pData) return NULL;

// third, copy the data.
switch(bitCount) {
case 8:
g_Rotate90<BYTE>(this, pRet);
break;
case 15:
case 16:
g_Rotate90<short>(this, pRet);
break;
case 24:
g_Rotate90<triple>(this, pRet);
break;
case 32:
g_Rotate90<long>(this, pRet);
break;
case 64:
g_Rotate90<double>(this, pRet);
break;
default:
assert(false);
delete pRet;
pRet = NULL;
}
return pRet;
}
2004-08-20 18:27:00 | [匿名:enoloo]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
Thanks a lot for sharing your code sample.
2004-08-20 07:06:00 | [匿名:lonelystranger]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
Good question for on-machine interviewing.
To me, it is somewhat like reverse a sentense or string. Is it true the interviewed will check 2 points:
1. in case >8bit bitmap, no special mem allocation will be used since it will be resolved by mem moving
2. make sure the guy not ignore the padding in bitmap.

I will be in Seatle tonight, 10 miles from those Remondian guys :=)
2004-08-19 21:49:00 | [匿名:codetiger]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
I see,studying
2004-08-19 10:35:00 | [匿名:holyeagle]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
[quote]
memcpy inside nested loop is slow. Do not use that in performance critical code. [/quote]

A very interesting piece of information. Thanks, Mr. Yuan!

Is it due to function call expense or some other reason? Could you elaborate on this issue?

Regards!
2004-08-19 01:04:00 | [匿名:ALNG]
#re: 面试问题: Rotate a bitmap by 90 degrees 编辑
Mr Yuan

Thanks for your advise! The crown of the year!

I have a question about performance of memcpy. As ehom(?!) mention, in Dev, if use compile r of gcc/g++, can convert memcpy to inline function, that will improve preformance of memory copy. I can't find the same way in VC, maybe is impossible in VC? if we can compile memcpy is inline function, I thought it wouldn't reduced performance. However, if it doesn't, I will write memory copy myself in asm as memcpy.

Thanks
2004-08-18 16:55:00 | [匿名:holyeagle]
对不起,目前本随笔不允许发表新评论.

Powered by: Joycode.MVC引擎 0.5.2.0