Hello,
as you all know we have quite a lot of regressions recently, and
recently they only add up to one another causing annoyances of
testers and developers. There is a strong need to change this
situation as soon as possible, otherwise the project's future is
undetermined.
I want to propose a step-by-step approach. Our brave testing team has
created a good overview of the most important regressions and bugs we
have so far: http://www.reactos.org/wiki/Buglist .
Now, very important(!):
For the first step I would like ALL developers to drop their current
ReactOS-related work, including all work in branches or wherever else
and focus ONLY on fixing regressions from that list. The goal is to
fix all confirmed regressions that have been introduced, starting
from the most recent and going down to the most ancient. All possible
ways to remove a regression could be used: starting from a proper fix
and finishing with a total revert or commenting out even good code.
Process coordination: feel free to commit proper fixes right away,
however as for reverts, I, and/or comittee of our core devs, would
like to have a final say on whether something should be reverted.
I repeat, all other non-regression related commits to the official
ReactOS SVN repository are forbidden, even to branches. The only
exception may be developers whose access is restricted to branches
only.
Thank you for understanding,
Aleksey Bragin.
Gregor Schneider <grschneider(a)gmail.com> wrote:
> Concerning using an internal format: let's assume we got three 8bpp
> surfaces (source, destination, pattern), which are quite big pending
> for a raster operation. You would convert these three to another
> format like 32bpp, process them and convert back?
No, of course not, Gregor,
You're absolutely right about the 8bpp S/D/P situation.
It would be a terrible idea to convert such a simple format to another
for a tertiary rop.
I think I was thinking about an internal graphic structure for the
display context .. ;-)
Accumulate all rops to an 32bit DIB internally, unless the physical
device is low bpp.
Come to think of it, almost anything but RLE is suitable internally,
depending
on the output color requirements. The graphics primitives for lines,
ellipses, and
so on can be small enough to separate a few specialized implementations for
different bit depths. I guess that's what You do ? (I didn't look yet).
Branching for different bit formats in the primitives would be a bad
idea though..
Wouldn't You agree ?
Best Regards
// Love
PS. I have a slice-line routine that's faster than an oiled lightning,
and it's really
small enough that it'd be worth replicating the code for different bit
depths.
Let me know if you're interested.
Hi guys,
Sorry for barging in on a discussion about code that's not on my desk,
but looking at your code there is something I want to point out.
Graphics code need to be *fast*, that's a primary consideration,
so I'm taken aback when I look at your inner bit expansion loop.
This is horrible from a standpoint of performance.
> length = (*bits++) >> shift;
> if (length)
> {
> c = *bits++;
> while (length--)
> {
> if (x >= width) break;
> temp = UncompressedBits + (((height - y) * Delta) + x);
> x++;
> *temp = c;
> }
> }
You're recomputing the start of the bit-run for every pixel you emit,
when you should move that calculation out of the inner loop to get
high performance. Graphics code is not the arena where you can be lazy
and hope that the optimizer will make your inefficient code faster.
At the very least You ought to write it something like this:
> length = (*bits++) >> shift;
> if (length)
> {
> c = *bits++;
> if (x + length > width) {
> // RLE encoding error - Bit-run exceeds width
> length = width - x;
> }
> temp = UncompressedBits + ((height - y) * Delta);
> x += length; // precompute finishing x
> while (length--)
> {
> *temp++ = c;
> }
> }
As a sideline note I'd like to mention that it's standard practice
in graphics libraries to use one unified bitmap format internally
to make internal processing like alpha-blend or text rendering or
whatever straight forward. The 32bit DIB format is quite suitable,
even if it uses the most memory - Graphics never was cheap on memory.
Just my penny to the pot
Best Regards
// Love
Hello,
This is the second time that work that someone on the ARM Team has worked on has been mostly reverted without any communication with us, and incorrect changes have been added to parts of our work.
The Eng function worked on even clearly stated comments such as "Compressed surfaces don't have scanlines!", yet this new patch restores the old incorrect functionality.
lDelta cannot be anything but != 0 for compressed data such as RLE or JPG/PNG! And creating the DIB should not decompress the bits.
Please read http://www.tech-archive.net/Archive/Development/microsoft.public.developmen… for example.
I do not understand how you can believe that "RLE" has a scan-line. I am not a graphics expert by no means, but even I understand this fact: by definition scan-lines in an RLE are dynamic, and a scan-line offset table contains information about that.
You do not have to take my word for it, as a simple driver test case will also show that Windows does not decompress RLE data or set lDelta to any other value than 0 with an RLE bitmap. You can find more information on RLE at http://www.fileformat.info/mirror/egff/ch09_03.htm.
If you are wondering why there have not been any commits coming lately, recent actions such as these as the cause.
-r
Author: khornicek
Date: Sat May 8 20:09:45 2010
New Revision: 47134
URL: http://svn.reactos.org/svn/reactos?rev=47134&view=rev
Log:
[WIN32K]
- Bring back support for RLE compressed bitmaps.
- Merge the decompress functions for 4bb and 8bpp bitmaps to one generic function.
- Simplify SURFMEM_bCreateDib a bit by not allowing PNG/JPEG compression at all.
See issue #5276 for more details.
Modified:
trunk/reactos/subsystems/win32/win32k/eng/surface.c
Modified: trunk/reactos/subsystems/win32/win32k/eng/surface.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/en…
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/eng/surface.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/eng/surface.c [iso-8859-1] Sat May 8 20:09:45 2010
@@ -204,48 +204,59 @@
return NewBitmap;
}
-VOID Decompress4bpp(SIZEL Size, BYTE *CompressedBits, BYTE *UncompressedBits, LONG Delta)
-{
- int x = 0;
- int y = Size.cy - 1;
- int c;
- int length;
- int width = ((Size.cx+1)/2);
- int height = Size.cy - 1;
+BOOL DecompressBitmap(SIZEL Size, BYTE *CompressedBits, BYTE *UncompressedBits, LONG Delta, ULONG Format)
+{
+ INT x = 0;
+ INT y = Size.cy - 1;
+ INT c;
+ INT length;
+ INT width;
+ INT height = Size.cy - 1;
BYTE *begin = CompressedBits;
BYTE *bits = CompressedBits;
BYTE *temp;
- while (y >= 0)
- {
- length = *bits++ / 2;
- if (length)
- {
- c = *bits++;
- while (length--)
- {
- if (x >= width) break;
- temp = UncompressedBits + (((height - y) * Delta) + x);
- x++;
- *temp = c;
- }
- }
- else
- {
- length = *bits++;
- switch (length)
- {
+ INT shift = 0;
+
+ if (Format == BMF_4RLE)
+ shift = 1;
+ else if(Format != BMF_8RLE)
+ return FALSE;
+
+ width = ((Size.cx + shift) >> shift);
+
+ _SEH2_TRY
+ {
+ while (y >= 0)
+ {
+ length = (*bits++) >> shift;
+ if (length)
+ {
+ c = *bits++;
+ while (length--)
+ {
+ if (x >= width) break;
+ temp = UncompressedBits + (((height - y) * Delta) + x);
+ x++;
+ *temp = c;
+ }
+ }
+ else
+ {
+ length = *bits++;
+ switch (length)
+ {
case RLE_EOL:
x = 0;
y--;
break;
case RLE_END:
- return;
+ _SEH2_YIELD(return TRUE);
case RLE_DELTA:
- x += (*bits++)/2;
- y -= (*bits++)/2;
+ x += (*bits++) >> shift;
+ y -= (*bits++) >> shift;
break;
default:
- length /= 2;
+ length = length >> shift;
while (length--)
{
c = *bits++;
@@ -260,69 +271,18 @@
{
bits++;
}
- }
- }
- }
-}
-
-VOID Decompress8bpp(SIZEL Size, BYTE *CompressedBits, BYTE *UncompressedBits, LONG Delta)
-{
- int x = 0;
- int y = Size.cy - 1;
- int c;
- int length;
- int width = Size.cx;
- int height = Size.cy - 1;
- BYTE *begin = CompressedBits;
- BYTE *bits = CompressedBits;
- BYTE *temp;
- while (y >= 0)
- {
- length = *bits++;
- if (length)
- {
- c = *bits++;
- while (length--)
- {
- if (x >= width) break;
- temp = UncompressedBits + (((height - y) * Delta) + x);
- x++;
- *temp = c;
- }
- }
- else
- {
- length = *bits++;
- switch (length)
- {
- case RLE_EOL:
- x = 0;
- y--;
- break;
- case RLE_END:
- return;
- case RLE_DELTA:
- x += *bits++;
- y -= *bits++;
- break;
- default:
- while (length--)
- {
- c = *bits++;
- if (x < width)
- {
- temp = UncompressedBits + (((height - y) * Delta) + x);
- x++;
- *temp = c;
- }
- }
- if ((bits - begin) & 1)
- {
- bits++;
- }
- }
- }
- }
+ }
+ }
+ }
+ }
+ _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+ {
+ DPRINT1("Decoding error\n");
+ _SEH2_YIELD(return FALSE);
+ }
+ _SEH2_END;
+
+ return TRUE;
}
HBITMAP FASTCALL
@@ -362,7 +322,7 @@
pso->cjBits = pso->lDelta * Size.cy;
UncompressedFormat = BMF_4BPP;
UncompressedBits = EngAllocMem(FL_ZERO_MEMORY, pso->cjBits, TAG_DIB);
- Decompress4bpp(Size, (BYTE *)Bits, (BYTE *)UncompressedBits, pso->lDelta);
+ DecompressBitmap(Size, (BYTE *)Bits, (BYTE *)UncompressedBits, pso->lDelta, Format);
}
else if (Format == BMF_8RLE)
{
@@ -370,7 +330,7 @@
pso->cjBits = pso->lDelta * Size.cy;
UncompressedFormat = BMF_8BPP;
UncompressedBits = EngAllocMem(FL_ZERO_MEMORY, pso->cjBits, TAG_DIB);
- Decompress8bpp(Size, (BYTE *)Bits, (BYTE *)UncompressedBits, pso->lDelta);
+ DecompressBitmap(Size, (BYTE *)Bits, (BYTE *)UncompressedBits, pso->lDelta, Format);
}
else
{
@@ -467,6 +427,7 @@
PSURFACE psurf;
SIZEL LocalSize;
BOOLEAN AllocatedLocally = FALSE;
+ PVOID DecompressedBits = NULL;
/*
* First, check the format so we can get the aligned scanline width.
@@ -500,17 +461,29 @@
break;
case BMF_8RLE:
+ ScanLine = (BitmapInfo->Width + 3) & ~3;
+ Compressed = TRUE;
+ break;
case BMF_4RLE:
+ ScanLine = ((BitmapInfo->Width + 7) & ~7) >> 1;
+ Compressed = TRUE;
+ break;
+
case BMF_JPEG:
case BMF_PNG:
- Compressed = TRUE;
- break;
+ ASSERT(FALSE); // ENGDDI shouldn't be creating PNGs for drivers ;-)
+ DPRINT1("No support for JPEG and PNG formats\n");
+ return NULL;
default:
DPRINT1("Invalid bitmap format\n");
return NULL;
}
+ /* Save local bitmap size */
+ LocalSize.cy = BitmapInfo->Height;
+ LocalSize.cx = BitmapInfo->Width;
+
/* Does the device manage its own surface? */
if (!Bits)
{
@@ -519,7 +492,8 @@
{
/* Note: we should not be seeing this scenario from ENGDDI */
ASSERT(FALSE);
- Size = BitmapInfo->Size;
+ DPRINT1("RLE compressed bitmap requested with no valid bitmap bits\n");
+ return NULL;
}
else
{
@@ -551,6 +525,22 @@
{
/* Should not have asked for user memory */
ASSERT((BitmapInfo->Flags & BMF_USERMEM) == 0);
+
+ if (Compressed)
+ {
+ DecompressedBits = EngAllocMem(FL_ZERO_MEMORY, BitmapInfo->Height * ScanLine, TAG_DIB);
+
+ if(!DecompressedBits)
+ return NULL;
+
+ if(!DecompressBitmap(LocalSize, (BYTE *)Bits, (BYTE *)DecompressedBits, ScanLine, BitmapInfo->Format))
+ {
+ EngFreeMem(DecompressedBits);
+ return NULL;
+ }
+
+ BitmapInfo->Format = (BitmapInfo->Format == BMF_4RLE) ? BMF_4BPP : BMF_8BPP;
+ }
}
/* Allocate the actual surface object structure */
@@ -564,6 +554,8 @@
else
EngFreeMem(Bits);
}
+ if (DecompressedBits)
+ EngFreeMem(DecompressedBits);
return NULL;
}
@@ -584,11 +576,9 @@
pso->fjBitmap = BitmapInfo->Flags & (BMF_TOPDOWN | BMF_UMPDMEM | BMF_USERMEM);
/* Save size and type */
- LocalSize.cy = BitmapInfo->Height;
- LocalSize.cx = BitmapInfo->Width;
pso->sizlBitmap = LocalSize;
pso->iType = STYPE_BITMAP;
-
+
/* Device-managed surface, no flags or dimension */
pso->dhsurf = 0;
pso->dhpdev = NULL;
@@ -599,48 +589,28 @@
psurf->hSecure = NULL;
psurf->hDIBSection = NULL;
psurf->flHooks = 0;
-
+
/* Set bits */
- pso->pvBits = Bits;
-
- /* Check for bitmap type */
- if (!Compressed)
- {
- /* Number of bits is based on the height times the scanline */
- pso->cjBits = BitmapInfo->Height * ScanLine;
- if (BitmapInfo->Flags & BMF_TOPDOWN)
- {
- /* For topdown, the base address starts with the bits */
- pso->pvScan0 = pso->pvBits;
- pso->lDelta = ScanLine;
- }
- else
- {
- /* Otherwise we start with the end and go up */
- pso->pvScan0 = (PVOID)((ULONG_PTR)pso->pvBits + pso->cjBits - ScanLine);
- pso->lDelta = -ScanLine;
- }
+ if(Compressed)
+ pso->pvBits = DecompressedBits;
+ else
+ pso->pvBits = Bits;
+
+ /* Number of bits is based on the height times the scanline */
+ pso->cjBits = BitmapInfo->Height * ScanLine;
+ if (BitmapInfo->Flags & BMF_TOPDOWN)
+ {
+ /* For topdown, the base address starts with the bits */
+ pso->pvScan0 = pso->pvBits;
+ pso->lDelta = ScanLine;
}
else
{
- /* Compressed surfaces don't have scanlines! */
- pso->lDelta = 0;
- pso->cjBits = BitmapInfo->Size;
-
- /* Check for JPG or PNG */
- if ((BitmapInfo->Format != BMF_JPEG) && (BitmapInfo->Format != BMF_PNG))
- {
- /* Wherever the bit data is */
- pso->pvScan0 = pso->pvBits;
- }
- else
- {
- /* Fancy formats don't use a base address */
- pso->pvScan0 = NULL;
- ASSERT(FALSE); // ENGDDI shouldn't be creating PNGs for drivers ;-)
- }
- }
-
+ /* Otherwise we start with the end and go up */
+ pso->pvScan0 = (PVOID)((ULONG_PTR)pso->pvBits + pso->cjBits - ScanLine);
+ pso->lDelta = -ScanLine;
+ }
+
/* Finally set the handle and uniq */
pso->hsurf = (HSURF)psurf->BaseObject.hHmgr;
pso->iUniq = 0;
Hello everybody,
We're currently in the process of moving a main server (Buildmaster,
ISO-Storage, etc.) to another location and doing upgrades on the other
servers.
As always, we try to keep the downtime low, but expect short outages in
the next few days.
You'll be notified when the move is over or if there are any major problems.
Best regards,
Colin
ekohl(a)svn.reactos.org wrote:
> PWSTR utf16_wcschr(PWSTR str, WCHAR c)
> +{
> + SIZE_T i;
> +
> + for(i = 0; str[i] && str[i] != c; i++);
> +
> + if(str[i])
> + return &str[i];
> + else
> + return NULL;
> +}
> +
> +PWSTR strchrW(PWSTR str, WCHAR c)
Why do you duplicate the same code for these wide-char string functions
here, just under a different name?
The utf16_* family of functions in this file was particularly designed to
address the issue of different wchar_t lengths on different hosts. It's
meant to be used together with the include/host/wcsfuncs.h header.
If you need an example, cmlib is one library using these functions (see
e.g. "cmlib.h" for the proper header inclusion, "cminit.c" for a use, etc.)
Best regards,
Colin
Hey guys,
Recently<http://source.winehq.org/git/wine.git/?a=history;f=dlls/msvcrt;hb=1ac163316…>wine
has made some improvements to its msvcrt and I've taken the time to
port them to reactos. changes include:
- implement per thread crt locales and many locale functions.
- implement/improve support for vc6, vc7, vc8 exceptions.
- implement/improve language support functions.
- implement/improve many library functions.
- implement many secure functions.
- use spec file for msvcrt.dll
- fix amd64 linking issues.
- add msvcirt.dll, msvcr7.dll ,msvc71.dll, msvc80.dll, msvcr90.dll from
wine (no longer need to install versions of ms runtime for visual c language
apps)
changes are divided into different parts but building has not been tested
with individual parts.
part 1, 2 and 3 include changes to centralize the definition of several key
macros (see reactos/wine/exception.h, reactos/wine/config.h,
crt/include/internal/wine/cppexcept.h)
also on part 2 in scanf.c wcs.c and possibly other files i didn't know what
libcntpr expected so i was quite severe hacking out functions and only
letting in the minimum necesary to build ntoskrnl and everything else that
uses it.
part3 includes changes to baseaddress.rbuild that should be revised, i
didn't know how to calculate a good base address for the msvcr dlls so i
just used the same one and let ntdll reallocate if necesary...
part5 applies to rostests.
and before you say it... msvcrt_winetest file now crashes with this trace:
dll/win32/kernel32/file/create.c:136 (CreateFileW@28) <-- arch didn't check
if lpFileName was valid
lib/sdk/crt/stdio/file.c:1485 (_wsopen)
modules/rostests/winetests/msvcrt/file.c:1179 (test_fopen_fclose_fcloseall)
ros-dev-request(a)reactos.org wrote:
>>> Look at this as a signal warning before the ship hits the reef.......
>>> The ship has turn into the reef!
>>>
>> No need to give false signals here, they often lead to loss of investment
>> (example from stock trading area).
>>
> Trying to make money from ReactOS is a bad idea and thinking about
> money from ReactOS, makes this idea delusional.
>
Sorry for barging into Your discussion guys,
but I think Aleksey meant that allegorically, not literally ??
Best Regards
Love
Michael!
Good work!!!!
Do you think this might be one of the reasons we are reentering
co_UserDestroyWindow? The DCE is cleared and (not cached) at first
then we go back a second time and hit the assert...?
http://www.reactos.org/bugzilla/show_bug.cgi?id=5320
Hey,
This one's mostly for Eric Kohl.
Newinflib seems to be causing some issues for usetup, accessing memory it
shouldn't.
Attached are some patches that solve this.
The newinflib one fixes some pointer arithmetic, which ended up lying about
a buffer size.
This was already fixed in the host code, so I just applied the same fix to
the ros code.
The usetup patch is a hackfix. The issue here is that usetup fetches a line
from a section,
but the line is blank. Usetup treats it as a data line, and reading the data
from it results
in NULL .. which usetup tries to dereference.
I just hackfixed that one because the code hasn't changed, so it must've
worked fine with
the previous inflib. So the question end up as: is newinflib wrong here by
not trying to skip
the blank line, or should usetup keep in mind such lines can exist.. or some
third option?
WBR,
Roel Messiant
PS: I know patches belong in bugzilla, but the perceived urgency on my side
won the decision on where to post them.
