Closed Bug 29879 Opened 25 years ago Closed 25 years ago

nsCSSRendering.cpp: #ifdef XP_Unix needs to go away

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
VERIFIED DUPLICATE of bug 33119

People

(Reporter: pierre, Assigned: pavlov)

References

Details

This bug is just a reminder... In nsCSSRendering.cpp, line 2195, you added some #ifdef XP_UNIX with the following comment: // XXX this #ifdef needs to go away when we are sure that this works on windows and mac
Putting myself as QA contact.
QA Contact: chrisd → pierre
can you test this on windows and mac? It should work fine and removing the ifdef should be OK.
If you provide a testcase along with 2 screen snapshots showing what should be displayed and what shouldn't depending on whether the code is enabled or not, then I'll gladly look at it but the fact is: a "#ifdef XP_something" has no place in Layout. - Why was it fixed on Unix only? Did it happen on all the flavors of Unix or just Linux? - Don't we have the same problem on Mac and Windows in other circumstances than the ones described in the original bug report? If not, why? - And more importantly, are you sure it is a problem of bad clipping in Layout instead of a problem with DrawImage() in LinuxGFX? If the problem is in LinuxGFX, we need to move your fix there. If the problem is in Layout, have you checked all the other instances of DrawImage() to see if we were clipping correctly? You should have done this verification before you checked in or eventually the day after, not mentioning asking for review and approval to dcone who is the original author of that part of the code. Sorry Pav.
Assignee: pavlov → dcone
pierre - the bug is in Mac and Windows GFX code. I had a long discussion with Don Cone on this issue and he told me to check this in #ifdef XP_UNIX until he had a chance to verify it was ok on windows and mac. He was going to make sure it worked on Mac and Windows... The problem in Windows and Mac GFX is that their drawing surfaces do not respect the clipping set by the rendering context. Since there is no way to set clipping directly on a drawing surface, and because DrawImage is a call on the rendering context, DrawImage should be using the clipping set. This problem occurs with new drawing surfaces. For example (as it does in the code in question), create new drawing surface on rendering context with clipping that you don't want to apply to the drawing surface because it would clip everything, then call DrawImage. On Mac and Windows this works because the new drawing surface has an empty clip rect and it does not draw using the clipping set on the rendering context as it should. On Linux, we do clip based on the clipping from the rendering context, so the DrawImage calls all get clipped by the clip rect previously set. The fix is to set the clipping rect before drawing the image on the new drawing surface. I hope this clears things up. The bug that this was causing before was the toolbar background on linux was not drawing properly. Stuart
Ok. You guys rock, I'll go to bed!
Status: NEW → ASSIGNED
When I took those comments out, windows broke. This bug will go away because the tiling is going to become platform specific (moved to the renderingcontexts). So I am marking this as invalid.. this will not happen.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → WONTFIX
Target Milestone: M16
this is still a bug and was in places like the old view manager. if it broke on windows, then windows needs to be fixed. Mac and Windows should not contain the broken behavior they have.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
If you have a bug for the platform-specific tiling code, you can put a ***BIG NOTE*** there to remember to remove the #ifdef XP_Unix stuff and then close the present bug. There is no need to keep a separate bug open about a code-level problem.
this bug should really be changed to say "creatin a new nsIDrawingSurface implimentation on mac and windows don't obey the clipping from the rendering context" I suppose. This should be fixed.
The clipping works the way it should on Mac and windows.. There is a default clipping on the Mac and Windows.. in the RenderingContext, not on the DrawingSurface.. which needs to have a RenderingContext if it is to have clipping, we don't want to have clipping being kept track of in two different classes.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → INVALID
What? That isn't the bug here. Mac and Windows should be using the clipping from the rendering context that created them when doing operations on the drawing surface. Drawing surface not having its own clipping information is the reason why it should be using the information from the rendering context that created it. We can not have differences in the way our platforms do rendering. I will go fix windows and mac if you like, but I would rather have someone that knows these platforms do it.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
The Mac and Windows drawingsurface both use the Clipping from the renderingContext which they are set into. We can only call drawing/blitting commands on the RenderingContext.. the final drawing into the drawingsurface is determined by the RenderingContext and which drawingsurface is set in the RenderingContext. The clipping is determined by this Renderingcontext and all drawing is clipped to this through the renderingcontext. So I think we agree to how it should work..and I am saying that this is currently how it does work on Mac and Windows. I am not sure how it works on Unix.. but that should also work this way.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → INVALID
I would have sided with Don just on the base of his explanations but I got confirmed in my feelings by looking at nsRenderingContextGTK.cpp. The function CopyOffScreenBits() ignores the flag NS_COPYBITS_USE_SOURCE_CLIP_REGION. Pav copied the code there with "#ifdef 0 // XXX impliment me" last October. It looks like the bug should be reopened and assigned to him.
I'm not honestly sure what that function is meant to do. I don't think that that flag is actually valid. Since the method takes a drawing surface as the source and the drawing surface does not have a clipping area, how can we use it's clipping?
This function copies bits from a source surface (passed as a parameter) to another one. If NS_COPYBITS_TO_BACK_BUFFER is 0, the destination surface is the screen. If it is 1, the destination surface is the surface currently selected in the RenderingContext. If NS_COPYBITS_USE_SOURCE_CLIP_REGION is 0, we clip to the region currently set in the RenderingContext. If it is 1, we clip to the region that was last set for the source surface (on the Mac, this region is part of the basic MacOS structure called the GrafPort, which is attached to all the drawing surfaces). As you can see, there is a clipping area attached to the drawing surface. What Don said is that all the clipping/drawing/blitting is done through the RenderingContext. After all, it really makes sense: that's how the MacOS and Windows work too, and I guess other platforms. Reopened.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Back to you, Pav.
Assignee: dcone → pavlov
Status: REOPENED → NEW
Mass-moving all M16 non-feature bugs to M17, which we still consider to be part of beta2
Target Milestone: M16 → M17
Marking dependency on bug 33119 which has been declared [nsbeta2+], but these bugs are duplicates really...
Depends on: 33119
nominating for nsbeta2. Pierre, if this is really a dup, please resolve as such
Keywords: nsbeta2
*** This bug has been marked as a duplicate of 33119 ***
Status: NEW → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → DUPLICATE
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.