Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3646)

Issue 5554059: On-surface brush

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 3 months ago by nicholasbishop
Modified:
12 years, 2 months ago
Reviewers:
bf-codereview, Jason.A.Wilkins, psy-fi
Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/
Visibility:
Public.

Description

This patch adds an optional brush display mode that visualizes the intersection between the brush's sphere of influence and the mesh. The code is based off Jason Wilkins' GSoC branch. The most significant change from the original code is that it does not require any copying or preservation of the GL depth buffer; instead the PBVH is used to partially redraw the necessary parts of the depth buffer. This could be a bit slower than making a copy of the depth buffer, but I think that relies too much on graphics cards/drivers behaving properly. Unchanged, however, is a requirement on the stencil buffer. Currently the patch unconditionally adds the stencil buffer to the GL-initialization code for all platforms in GHOST. (I've only tested X11 here, the Mac and Win code I just copied over.) I'm not sure if this is considered safe, or if we should have a compile-time option, or a command-line startup option, or better run-time checking (i.e. if drawable request fails, create a new request without the stencil buffer bit.) Or some combination of these.

Patch Set 1 #

Total comments: 5

Patch Set 2 : Fix texture overlay and update comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+290 lines, -18 lines) Patch
intern/ghost/intern/GHOST_WindowCarbon.cpp View 1 2 chunks +2 lines, -0 lines 0 comments Download
intern/ghost/intern/GHOST_WindowCocoa.mm View 1 2 chunks +6 lines, -0 lines 0 comments Download
intern/ghost/intern/GHOST_WindowWin32.cpp View 1 2 chunks +3 lines, -2 lines 0 comments Download
intern/ghost/intern/GHOST_WindowX11.cpp View 1 1 chunk +1 line, -0 lines 0 comments Download
release/scripts/startup/bl_ui/space_view3d_toolbar.py View 1 1 chunk +1 line, -0 lines 0 comments Download
source/blender/blenkernel/intern/paint.c View 1 1 chunk +1 line, -1 line 0 comments Download
source/blender/editors/include/ED_view3d.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
source/blender/editors/sculpt_paint/paint_cursor.c View 1 5 chunks +227 lines, -7 lines 0 comments Download
source/blender/editors/space_view3d/view3d_draw.c View 1 4 chunks +4 lines, -4 lines 0 comments Download
source/blender/gpu/GPU_draw.h View 1 1 chunk +4 lines, -0 lines 0 comments Download
source/blender/gpu/intern/gpu_draw.c View 1 1 chunk +10 lines, -0 lines 0 comments Download
source/blender/makesdna/DNA_scene_types.h View 1 1 chunk +1 line, -1 line 0 comments Download
source/blender/makesrna/intern/rna_sculpt_paint.c View 1 3 chunks +27 lines, -3 lines 0 comments Download

Messages

Total messages: 7
nicholasbishop
12 years, 3 months ago (2012-01-19 05:24:54 UTC) #1
psy-fi
Did a first review on the OpenGL code with some comments. Looks OK to me. ...
12 years, 3 months ago (2012-01-19 12:55:07 UTC) #2
psy-fi
Small correction, also an extra observation, this method may fail if the brush sphere is ...
12 years, 3 months ago (2012-01-19 13:04:31 UTC) #3
nicholasbishop
Fix texture overlay and update comment
12 years, 2 months ago (2012-02-13 19:21:04 UTC) #4
nicholasbishop
On 2012/02/13 19:21:04, nicholasbishop wrote: > Fix texture overlay and update comment A couple updates: ...
12 years, 2 months ago (2012-02-13 19:30:10 UTC) #5
nicholasbishop
Sergey noted on IRC that the on-surface brush can look bad with the layer brush. ...
12 years, 2 months ago (2012-02-13 21:36:08 UTC) #6
Jason.A.Wilkins
12 years, 2 months ago (2012-02-16 21:54:32 UTC) #7
I had z-fighting show up on some platforms when using this approach to draw the
"wrap" mode overlay (where the texture map is shown on the surface).

I would not be afraid of the z-buffer approach since it is a pretty fundamental
part of current graphics APIs and shouldn't be broken.  

However I do not see anything wrong with removing that requirement if z-fighting
is not a problem.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b