Created attachment 78747 [details]
Fix patch for Bug 120165

The sample file contains .svm files which contains bitmapex whose mask is not the same size as its bitmap. And this cause access out of boundary in GraphicManager::ImplCreateScaled

Comment on attachment 78747 [details]
Fix patch for Bug 120165

update a patch with full source file path later

Created attachment 78748 [details]
Fix patch for Bug 120165

Update fix patch

@pengyunquan: I have two questions regarding the fix:
1. Instead of introducing two new variables nDstWV and nDstHV, would it be possible to modify and reuse the existing ones nDstW and nDstH?

2. The use pMapLX and pMapLY in lines 1251 and 1252 indicates that their values are sorted.  Is that true?

ALG: I have even more questions to this crash and fix.
- Why only fix for Mask, BitmapEx can also have Alpha with the same problem?
- Why not fix in a way that alpha/mask gets scaled and not just truncated?
- The BitmapEx constructor already asserts unequal sizes, this imples that unequal sizes are not wanted from design. Why not fix there?

I will propose an other xix/patch.

Created attachment 78754 [details]
Fix by correcting Mask/Alpha at BitmapEx construction time

ALG: Patch added which corrects given Alpha/Mask at BitmapEx construction time, thus ensuring unequal pixel sizes will not happen. This makes all code based on BitmapEx pixel magic involving Alpha/Mask more simple and safe.

@ALG: Is this an additional patch or does it replace the previous one?

@pengyunquan: Is the second patch OK for you?

ALG: It replaces the original one. It fixes the general problem root by adapting the mask/alpha bitmap at BitmapEx construction time if not the same pixel size (there was an assert there already indicating that this is not wanted).

Then go ahead and check in your changes.

"alg" committed SVN revision 1366405 into trunk:
#120165# Adapt Mask/Alpha at BitmapEx construction when size differs from bas...

ALG: Okay, comitted as revision 1366405, done.

I have one question: If we scale the mask bitmap, is the mask bitmap still a valid mask? As we know even a pixel offset of mask bitmap will get quite different mask result.

If we scale the mask bitmap, we changed the saved BitmapEx object. I know it is an abnormal BitmapEx object, and we don't know how this abnormal object is generated. But I think we'd better keep the abnormal BitmapEx object to avoid risk of incorrect display result.

ALG->pengyunquan: When the mask has not the same pixel size, it can anyways not fit the content; the mask will anyways been scaled when painting with VCL (not truncated as in the patch), so scaling it at BitmapEx construction time makes no difference for visualization, except that the visualisation does the scaling at runtime.
Changing the saved BitmapEx: Yes, it will be saved changed, but I think this is acceptable for such bitmaps. As explained above, VCL will scale the Mask/Alpha at output to the same output pixel size anyways. All pixel formats I know which support Alpha/Mask have something like RGBA anyways in file formats, thus a unequal SizePixel is not even possible/allowed since one alpha value is bound to one pixel.
I also do not know how that BitmapEx was created, but as the existing asserts show it is not intended and leads to many exceptions in the whole office. In many places are asserts to warn in this case (as in the task case there were already warnings), but no handling implemented. I have not designed VCL, but this looks like purpose, too.
If there should really be a case where this is problematic we can react on it when it comes up.

verified on XP SP3 and Ubuntu 12.04 against trunk build 1374181

Verified on xp, Ubuntu, win7,mac,win7-64,Vista,suse,redhat, SLED 11 SP1 64 bit, all pass, close this bug

set Target Milestone to AOO 3.5.0 for PM purpose.