Merge branch 'master' into lighthouse2

# Conflicts:
#	topics/light.py -> resolved

active_projects/WindingNumber.py

@@ -67,10 +67,14 @@ class EquationSolver1d(GraphScene, ZoomedScene):
             self.add(target_line_label)
 
     def solveEquation(self):
-        leftBrace, rightBrace = xBraces = TexMobject("||")
+        leftBrace = TexMobject("[")
+        rightBrace = TexMobject("]")
+        xBraces = Group(leftBrace, rightBrace)
         xBraces.stretch(2, 0)
 
-        downBrace, upBrace = yBraces = TexMobject("||")
+        downBrace = TexMobject("[")
+        upBrace = TexMobject("]")
+        yBraces = Group(downBrace, upBrace)
         yBraces.stretch(2, 0)
         yBraces.rotate(TAU/4)
 
@@ -79,7 +83,7 @@ class EquationSolver1d(GraphScene, ZoomedScene):
         upperX = self.initial_upper_x
         upperY = self.func(upperX)
 
-        leftBrace.move_to(self.coords_to_point(lowerX, 0))
+        leftBrace.move_to(self.coords_to_point(lowerX, 0), aligned_edge = LEFT)
         leftBraceLabel = DecimalNumber(lowerX)
         leftBraceLabel.next_to(leftBrace, DOWN + LEFT, buff = SMALL_BUFF)
         leftBraceLabelAnimation = ContinualChangingDecimal(leftBraceLabel, 
@@ -87,7 +91,7 @@ class EquationSolver1d(GraphScene, ZoomedScene):
             tracked_mobject = leftBrace)
         self.add(leftBraceLabelAnimation)
         
-        rightBrace.move_to(self.coords_to_point(upperX, 0))
+        rightBrace.move_to(self.coords_to_point(upperX, 0), aligned_edge = RIGHT)
         rightBraceLabel = DecimalNumber(upperX)
         rightBraceLabel.next_to(rightBrace, DOWN + RIGHT, buff = SMALL_BUFF)
         rightBraceLabelAnimation = ContinualChangingDecimal(rightBraceLabel, 
@@ -95,7 +99,7 @@ class EquationSolver1d(GraphScene, ZoomedScene):
             tracked_mobject = rightBrace)
         self.add(rightBraceLabelAnimation)
 
-        downBrace.move_to(self.coords_to_point(0, lowerY))
+        downBrace.move_to(self.coords_to_point(0, lowerY), aligned_edge = DOWN)
         downBraceLabel = DecimalNumber(lowerY)
         downBraceLabel.next_to(downBrace, LEFT + DOWN, buff = SMALL_BUFF)
         downBraceLabelAnimation = ContinualChangingDecimal(downBraceLabel, 
@@ -103,7 +107,7 @@ class EquationSolver1d(GraphScene, ZoomedScene):
             tracked_mobject = downBrace)
         self.add(downBraceLabelAnimation)
         
-        upBrace.move_to(self.coords_to_point(0, upperY))
+        upBrace.move_to(self.coords_to_point(0, upperY), aligned_edge = UP)
         upBraceLabel = DecimalNumber(upperY)
         upBraceLabel.next_to(upBrace, LEFT + UP, buff = SMALL_BUFF)
         upBraceLabelAnimation = ContinualChangingDecimal(upBraceLabel, 
@@ -200,7 +204,7 @@ class EquationSolver1d(GraphScene, ZoomedScene):
         self.drawGraph()
         self.solveEquation()
 
-colorslist = map(color_to_rgba, ["#FF0000", ORANGE, YELLOW, "#00FF00", "#0000FF", "#FF00FF"])
+colorslist = map(color_to_rgba, ["#FF0000", "#FFFF00", "#00FF00", "#0000FF"])
 
 def rev_to_rgba(alpha):
     alpha = alpha % 1
@@ -330,9 +334,7 @@ def point_func_from_complex_func(f):
 
 test_map_func = point_func_from_complex_func(lambda c: c**2)
 
-empty_animation = Animation(Mobject(), run_time = 0)
+empty_animation = EmptyAnimation()
-def EmptyAnimation():
-    return empty_animation
 
 class WalkerAnimation(Animation):
     CONFIG = {
@@ -347,6 +349,8 @@ class WalkerAnimation(Animation):
         self.rev_func = rev_func
         self.coords_to_point = coords_to_point
         self.compound_walker = VGroup()
+        self.show_arrows = show_arrows
+
         base_walker = Dot().scale(5) # PiCreature().scale(0.8) # 
         self.compound_walker.walker = base_walker.scale(0.35).set_stroke(BLACK, 1.5) #PiCreature()
         if show_arrows:
@@ -365,7 +369,7 @@ class WalkerAnimation(Animation):
         self.mobject.shift(cur_point - self.mobject.walker.get_center())
         rev = self.rev_func(cur_coords)
         self.mobject.walker.set_fill(rev_to_color(rev))
-        if show_arrows:
+        if self.show_arrows:
             self.mobject.arrow.set_fill(rev_to_color(rev))
             self.mobject.arrow.rotate(
                 rev * TAU, 
@@ -430,12 +434,21 @@ class ColorMappedByFuncScene(Scene):
     CONFIG = {
         "func" : lambda p : p,
         "num_plane" : NumberPlane(),
-        "display_output_color_map" : False
+        "show_num_plane" : True,
+
+        "show_output" : False, # Not currently implemented; TODO
     }
 
-    def construct(self):
+    def setup(self):
-        display_func = self.func if not self.display_output_color_map else lambda p : p
+        if self.show_output:
+            self.pos_func = self.func
+        else:
+            self.pos_func = lambda p : p
 
+    def construct(self):
+        display_func = self.func if not self.show_output else lambda p : p
+
+        if self.show_num_plane:
             self.num_plane.fade()
             self.add(self.num_plane)
         self.camera.set_background_from_func(
@@ -449,11 +462,15 @@ class ColorMappedByFuncScene(Scene):
             )
         )
 
+class ColorMappedByFuncStill(ColorMappedByFuncScene):
+    def construct(self):
+        ColorMappedByFuncScene.construct(self)
+        self.wait()
+
 class PiWalker(ColorMappedByFuncScene):
     CONFIG = {
         "walk_coords" : [],
         "step_run_time" : 1,
-        "show_arrows" : True
     }
 
     def construct(self):
@@ -476,7 +493,7 @@ class PiWalker(ColorMappedByFuncScene):
                     coords_to_point = num_plane.coords_to_point,
                     rev_func = rev_func,
                     remover = (i < len(walk_coords) - 1),
-                    show_arrows = self.show_arrows
+                    show_arrows = not self.show_output
                 ),
                 run_time = self.step_run_time)
 
@@ -515,9 +532,7 @@ class PiWalkerCircle(PiWalker):
         self.walk_coords = [r * np.array((np.cos(i * TAU/N), np.sin(i * TAU/N))) for i in range(N)]
         PiWalker.setup(self)
 
-# TODO: Perhaps restructure this to avoid using AnimationGroup, and instead 
+# TODO: Give drawn lines a bit of buffer, so that the rectangle's corners are filled in
-# use lists of animations or lists or other such data, to be merged and processed into parallel 
-# animations later
 class EquationSolver2d(ColorMappedByFuncScene):
     CONFIG = {
         "initial_lower_x" : -5.1,
@@ -526,7 +541,8 @@ class EquationSolver2d(ColorMappedByFuncScene):
         "initial_upper_y" : 3.1,
         "num_iterations" : 5,
         "num_checkpoints" : 10,
-        "display_in_parallel" : True
+        "display_in_parallel" : True,
+        "use_fancy_lines" : True,
         # TODO: Consider adding a "find_all_roots" flag, which could be turned off 
         # to only explore one of the two candidate subrectangles when both are viable
     }
@@ -534,23 +550,32 @@ class EquationSolver2d(ColorMappedByFuncScene):
     def construct(self):
         ColorMappedByFuncScene.construct(self)
         num_plane = self.num_plane
+        self.remove(num_plane)
+
+        background = self.camera.background
+        self.camera.init_background() # Clearing background
 
         rev_func = lambda p : point_to_rev(self.func(p))
         clockwise_rev_func = lambda p : -rev_func(p)
 
-        def Animate2dSolver(cur_depth, rect, dim_to_split):
+        def Animate2dSolver(cur_depth, rect, dim_to_split, sides_to_draw = [0, 1, 2, 3]):
             print "Solver at depth: " + str(cur_depth)
 
             if cur_depth >= self.num_iterations:
-                return EmptyAnimation()
+                return empty_animation
 
-            def draw_line_return_wind(start, end, start_wind, should_linger = False):
+            def draw_line_return_wind(start, end, start_wind, should_linger = False, draw_line = True):
                 alpha_winder = make_alpha_winder(clockwise_rev_func, start, end, self.num_checkpoints)
                 a0 = alpha_winder(0)
                 rebased_winder = lambda alpha: alpha_winder(alpha) - a0 + start_wind
-                thin_line = Line(num_plane.coords_to_point(*start), num_plane.coords_to_point(*end),
+                thick_line = Line(num_plane.coords_to_point(*start), num_plane.coords_to_point(*end),
-                    stroke_width = 2,
+                    stroke_width = 10,
                     color = RED)
+                if self.use_fancy_lines:
+                    colored_line = BackgroundColoredVMobject(thick_line, background_image_file = None)
+                    colored_line.set_background_array(background)
+                else:
+                    colored_line = thick_line.set_stroke_with(4)
 
                 walker_anim = LinearWalker(
                     start_coords = start, 
@@ -566,14 +591,16 @@ class EquationSolver2d(ColorMappedByFuncScene):
                 else:
                     rate_func = None
 
+                opt_line_anim = ShowCreation(colored_line) if draw_line else empty_animation
+
                 line_draw_anim = AnimationGroup(
-                    ShowCreation(thin_line), 
+                    opt_line_anim, 
                     walker_anim,
                     rate_func = rate_func)
                 return (line_draw_anim, rebased_winder(1))
 
             wind_so_far = 0
-            anim = EmptyAnimation()
+            anim = empty_animation
             sides = [
                 rect.get_top(), 
                 rect.get_right(), 
@@ -582,7 +609,8 @@ class EquationSolver2d(ColorMappedByFuncScene):
             ]
             for (i, (start, end)) in enumerate(sides):
                 (next_anim, wind_so_far) = draw_line_return_wind(start, end, wind_so_far, 
-                    should_linger = i == len(sides) - 1)
+                    should_linger = i == len(sides) - 1,
+                    draw_line = i in sides_to_draw)
                 anim = Succession(anim, next_anim)
 
             total_wind = round(wind_so_far)
@@ -594,32 +622,36 @@ class EquationSolver2d(ColorMappedByFuncScene):
                     rect.get_bottom_right(), 
                     rect.get_bottom_left()
                 ]
-                points = [num_plane.coords_to_point(x, y) for (x, y) in coords]
+                points = np.array([num_plane.coords_to_point(x, y) for (x, y) in coords]) + IN
                 # TODO: Maybe use diagonal lines or something to fill in rectangles indicating
                 # their "Nothing here" status?
-                fill_rect = polygonObject = Polygon(*points, fill_opacity = 0.8, color = RED)
+                # Or draw a large X or something
+                fill_rect = polygonObject = Polygon(*points, fill_opacity = 0.8, color = GREY)
                 return Succession(anim, FadeIn(fill_rect))
             else:
                 (sub_rect1, sub_rect2) = rect.splits_on_dim(dim_to_split)
-                sub_rects = [sub_rect1, sub_rect2]
+                if dim_to_split == 0:
+                    sub_rect_and_sides = [(sub_rect1, 1), (sub_rect2, 3)]
+                else:
+                    sub_rect_and_sides = [(sub_rect1, 2), (sub_rect2, 0)]
                 sub_anims = [
                     Animate2dSolver(
                         cur_depth = cur_depth + 1,
                         rect = sub_rect,
-                        dim_to_split = 1 - dim_to_split
+                        dim_to_split = 1 - dim_to_split,
+                        sides_to_draw = [side_to_draw]
                     )
-                    for sub_rect in sub_rects
+                    for (sub_rect, side_to_draw) in sub_rect_and_sides
                 ]
                 mid_line_coords = rect.split_line_on_dim(dim_to_split)
                 mid_line_points = [num_plane.coords_to_point(x, y) for (x, y) in mid_line_coords]
-                mid_line = DashedLine(*mid_line_points) # TODO: Have this match rectangle line style, apart from dashes and thin-ness?
+                # TODO: Have this match rectangle line style, apart from dashes and thin-ness?
-                if len(sub_anims) > 0:
+                # Though there is also informational value in seeing the dashed line separately from rectangle lines
+                mid_line = DashedLine(*mid_line_points)
                 if self.display_in_parallel:
                     recursive_anim = AnimationGroup(*sub_anims) 
                 else:
                     recursive_anim = Succession(*sub_anims)
-                else:
-                    recursive_anim = empty_animation # Have to do this because Succession doesn't currently handle empty animations
                 return Succession(anim, 
                     ShowCreation(mid_line),
                     recursive_anim
@@ -780,7 +812,7 @@ class FirstSqrtScene(EquationSolver1d):
         "targetY" : 2,
         "initial_lower_x" : 1,
         "initial_upper_x" : 2,
-        "num_iterations" : 10,
+        "num_iterations" : 3,
         "iteration_at_which_to_start_zoom" : 3,
         "graph_label" : "y = x^2",
         "show_target_line" : True,
@@ -1096,8 +1128,8 @@ class LoopSplitSceneMapped(LoopSplitScene):
 class FundThmAlg(EquationSolver2d):
     CONFIG = {
         "func" : plane_poly_with_roots((1, 2), (-1, 1.5), (-1, 1.5)),
-        "num_iterations" : 4,
+        "num_iterations" : 5,
-        "display_in_parallel" : False
+        "display_in_parallel" : True
     }
 
 # TODO: Borsuk-Ulam visuals
@@ -1159,11 +1191,23 @@ class DiffOdometer(OdometerScene):
 
 ####################
 
+# Random test scenes and test functions go here:
+
+def rect_to_circle((x, y, z)):
+    size = np.sqrt(x**2 + y**2)
+    max_abs_size = max(abs(x), abs(y))
+    return fdiv(np.array((x, y, z)) * max_abs_size, size)
+
 class MapPiWalkerRect(PiWalkerRect):
     CONFIG = {
         "camera_class" : MappingCamera,
-        "camera_config" : {"mapping_func" : test_map_func},
+        "camera_config" : {"mapping_func" : rect_to_circle},
         "display_output_color_map" : True
     }
 
+class ShowBack(PiWalkerRect):
+    CONFIG = {
+         "func" : plane_poly_with_roots((1, 2), (-1, 1.5), (-1, 1.5))
+    }
+
 # FIN

animation/animation.py

@@ -22,6 +22,9 @@ class Animation(object):
         #one_at_a_time, all_at_once
         "submobject_mode" : "all_at_once",
         "lag_factor" : 2,
+        # Used by EmptyAnimation to announce itself ignorable
+        # in Successions and AnimationGroups
+        "empty" : False
     }
     def __init__(self, mobject, **kwargs):
         mobject = instantiate(mobject)

animation/simple_animations.py

@@ -403,7 +403,7 @@ class Succession(Animation):
         for anim in animations:
             anim.update(0)
 
-        animations = filter (lambda x : x.run_time != 0, animations)
+        animations = filter (lambda x : not(x.empty), animations)
 
         self.run_times = [anim.run_time for anim in animations]
         if "run_time" in kwargs:
@@ -411,6 +411,8 @@ class Succession(Animation):
         else:
             run_time = sum(self.run_times)
         self.num_anims = len(animations)
+        if self.num_anims == 0:
+            self.empty = True
         self.animations = animations
         #Have to keep track of this run_time, because Scene.play
         #might very well mess with it.
@@ -439,10 +441,10 @@ class Succession(Animation):
 
         Animation.__init__(self, self.mobject, run_time = run_time, **kwargs)
 
-    # Beware: This does NOT take care of updating the subanimation to 0
+    # Beware: This does NOT take care of calling update(0) on the subanimation.
     # This was important to avoid a pernicious possibility in which subanimations were called
     # with update(0) twice, which could in turn call a sub-Succession with update(0) four times,
-    # continuing exponentially
+    # continuing exponentially.
     def jump_to_start_of_anim(self, index):
         if index != self.current_anim_index:
             self.mobject.remove(*self.mobject.submobjects) # Should probably have a cleaner "remove_all" method...
@@ -485,6 +487,12 @@ class AnimationGroup(Animation):
     }
     def __init__(self, *sub_anims, **kwargs):
         digest_config(self, kwargs, locals())
+        sub_anims = filter (lambda x : not(x.empty), sub_anims)
+        if len(sub_anims) == 0:
+            self.empty = True
+            self.run_time = 0
+        else:
+            # Should really make copies of animations, instead of messing with originals...
             sync_animation_run_times_and_rate_funcs(*sub_anims, **kwargs)
             self.run_time = max([a.run_time for a in sub_anims])
         everything = Mobject(*[a.mobject for a in sub_anims])
@@ -497,3 +505,12 @@ class AnimationGroup(Animation):
     def clean_up(self, *args, **kwargs):
         for anim in self.sub_anims:
             anim.clean_up(*args, **kwargs)
+
+class EmptyAnimation(Animation):
+    CONFIG = {
+        "run_time" : 0,
+        "empty" : True
+    }
+
+    def __init__(self, *args, **kwargs):
+        return Animation.__init__(self, Group(), *args, **kwargs)

camera/camera.py

@@ -99,7 +99,7 @@ class Camera(object):
     def set_background(self, pixel_array, convert_from_floats = False):
         self.background = self.convert_pixel_array(pixel_array, convert_from_floats)
 
-    def set_background_from_func(self, coords_to_colors_func):
+    def make_background_from_func(self, coords_to_colors_func):
         """
         Sets background by using coords_to_colors_func to determine each pixel's color. Each input 
         to coords_to_colors_func is an (x, y) pair in space (in ordinary space coordinates; not 
@@ -114,9 +114,10 @@ class Camera(object):
             2,
             coords
         )
-        self.set_background(new_background, convert_from_floats = True)
+        return self.convert_pixel_array(new_background, convert_from_floats = True)
 
-        print "Ending set_background_from_func"
+    def set_background_from_func(self, coords_to_colors_func):
+        self.set_background(self.make_background_from_func(coords_to_colors_func))
 
     def reset(self):
         self.set_pixel_array(self.background)
@@ -140,7 +141,8 @@ class Camera(object):
         self, mobjects, 
         include_submobjects = True,
         excluded_mobjects = None,
-        z_buff_func = lambda m : m.get_center()[2]
+        #Round z coordinate to nearest hundredth when comparring
+        z_buff_func = lambda m : np.round(m.get_center()[2], 2)
         ):
         if include_submobjects:
             mobjects = self.extract_mobject_family_members(
@@ -152,6 +154,9 @@ class Camera(object):
                 )
                 mobjects = list_difference_update(mobjects, all_excluded)
 
+        # Should perhaps think about what happens here when include_submobjects is False,
+        # (for now, the onus is then on the caller to ensure this is handled correctly by
+        # passing us an appropriately pre-flattened list of mobjects if need be)
         return sorted(mobjects, lambda a, b: cmp(z_buff_func(a), z_buff_func(b)))
 
     def capture_mobject(self, mobject, **kwargs):
@@ -497,7 +502,8 @@ class MovingCamera(Camera):
             0 if self.aligned_dimension == "height" else 1
         )
 
-
+# TODO: Add an attribute to mobjects under which they can specify that they should just 
+# map their centers but remain otherwise undistorted (useful for labels, etc.)
 class MappingCamera(Camera):
     CONFIG = {
         "mapping_func" : lambda p : p,

constants.py

@@ -60,6 +60,7 @@ LEFT_SIDE  = SPACE_WIDTH*LEFT
 RIGHT_SIDE = SPACE_WIDTH*RIGHT
 
 TAU = 2*np.pi
+DEGREES = TAU/360
 
 # Change this to point to where you want 
 # animation files to output

helpers.py

@@ -199,14 +199,16 @@ def bezier(points):
 def remove_list_redundancies(l):
     """
     Used instead of list(set(l)) to maintain order
+    Keeps the last occurance of each element
     """
-    result = []
+    reversed_result = []
     used = set()
-    for x in l:
+    for x in reversed(l):
         if not x in used:
-            result.append(x)
+            reversed_result.append(x)
             used.add(x)
-    return result
+    reversed_result.reverse()
+    return reversed_result
 
 def list_update(l1, l2):
     """
@@ -544,12 +546,16 @@ def wiggle(t, wiggles = 2):
 
 def squish_rate_func(func, a = 0.4, b = 0.6):
     def result(t):
+        if a == b:
+            return a
+
         if t < a:
             return func(0)
         elif t > b:
             return func(1)
         else:
             return func((t-a)/(b-a))
+            
     return result
 
 # Stylistically, should this take parameters (with default values)?
@@ -649,8 +655,8 @@ def angle_between_vectors(v1, v2):
     return np.arccos(np.dot(v1,v2)/(l1*l2))
 
 def project_along_vector(point, vector):
-    matrix = np.eye(3) - np.outer(vector,vector)
+    matrix = np.identity(3) - np.outer(vector, vector)
-    return np.dot(point,matrix.T)
+    return np.dot(point, matrix.T)
 
 def concatenate_lists(*list_of_lists):
     return [item for l in list_of_lists for item in l]

mobject/vectorized_mobject.py

@@ -459,7 +459,8 @@ class VectorizedPoint(VMobject):
 
 class BackgroundColoredVMobject(VMobject):
     CONFIG = {
-        "background_image" : "color_background",
+        # Can be set to None, using set_background_array to initialize instead
+        "background_image_file" : "color_background",
         "stroke_color" : WHITE,
         "fill_color" : WHITE,
     }
@@ -475,10 +476,14 @@ class BackgroundColoredVMobject(VMobject):
         for submob in vmobject.submobjects:
             self.add(BackgroundColoredVMobject(submob, **kwargs))
 
+        if self.background_image_file != None:
             #Initialize background array
-        path = get_full_raster_image_path(self.background_image)
+            path = get_full_raster_image_path(self.background_image_file)
             image = Image.open(path)
-        self.background_array = np.array(image)
+            self.set_background_array(np.array(image))
+
+    def set_background_array(self, background_array):
+        self.background_array = background_array
 
     def resize_background_array(self, new_width, new_height, mode = "RGBA"):
         image = Image.fromarray(self.background_array, mode = mode)

topics/light.py

@@ -42,14 +42,16 @@ inverse_quadratic = lambda maxint,scale,cutoff: inverse_power_law(maxint,scale,c
 
 
 
+# Note: Overall, this class seems perfectly reasonable to me, the main 
+# thing to be wary of is that calling self.add(submob) puts that submob
+# at the end of the submobjects list, and hence on top of everything else
+# which is why the shadow might sometimes end up behind the spotlight
 class LightSource(VMobject):
-
     # combines:
     # a lighthouse
     # an ambient light
     # a spotlight
     # and a shadow
-
     CONFIG = {
         "source_point": ORIGIN,
         "color": LIGHT_COLOR,
@@ -115,6 +117,8 @@ class LightSource(VMobject):
         if self.has_screen():
             self.spotlight.screen = new_screen
         else:
+            # Note: See below
+            # index = self.submobjects.index(self.spotlight)
             self.remove(self.spotlight)
             self.spotlight = Spotlight(
                 source_point = self.source_point,
@@ -124,6 +128,13 @@ class LightSource(VMobject):
                 screen = new_screen
             )
             self.spotlight.move_source_to(self.source_point)
+
+            # Note: This line will make spotlight show up at the end
+            # of the submojects list, which can make it show up on
+            # top of the shadow. To make it show up in the
+            # same spot, you could try the following line, 
+            # where "index" is what I defined above:
+            # self.submobjects.insert(index, self.spotlight)
             self.add(self.spotlight)
         
         # in any case
@@ -133,9 +144,12 @@ class LightSource(VMobject):
 
 
     def move_source_to(self,point):
-
         apoint = np.array(point)
         v = apoint - self.source_point
+        # Note: As discussed, things stand to behave better if source
+        # point is a submobject, so that it automatically interpolates
+        # during an animation, and other updates can be defined wrt 
+        # that source point's location
         self.source_point = apoint
         self.lighthouse.next_to(apoint,DOWN,buff = 0)
         self.ambient_light.move_source_to(apoint)
@@ -144,12 +158,6 @@ class LightSource(VMobject):
         self.update()
         return self
         
-
-
-
-
-
-        
     def set_radius(self,new_radius):
         self.radius = new_radius
         self.ambient_light.radius = new_radius
@@ -170,12 +178,13 @@ class LightSource(VMobject):
             projected_screen_points.append(self.spotlight.project(point))
 
 
-
-
         projected_source = project_along_vector(self.source_point,self.spotlight.projection_direction())
 
-        projected_point_cloud_3d = np.append(projected_screen_points,
+        projected_point_cloud_3d = np.append(
-            np.reshape(projected_source,(1,3)),axis = 0)
+            projected_screen_points,
+            np.reshape(projected_source,(1,3)),
+            axis = 0
+        )
         rotation_matrix = z_to_vector(self.spotlight.projection_direction())
         back_rotation_matrix = rotation_matrix.T # i. e. its inverse
 
@@ -222,6 +231,9 @@ class LightSource(VMobject):
         new_anchors = np.append(new_anchors,anchors[index:],axis = 0)
         self.shadow.set_points_as_corners(new_anchors)
 
+        # Note: Theoretically this should not be necessary as long as we make
+        # sure the shadow shows up after the spotlight in the submobjects list.
+        # 
         # shift it closer to the camera so it is in front of the spotlight
         self.shadow.shift(1e-5*self.spotlight.projection_direction())
         self.shadow.mark_paths_closed = True
@@ -292,6 +304,10 @@ class AmbientLight(VMobject):
 
         # in theory, this method is only called once, right?
         # so removing submobs shd not be necessary
+        # 
+        # Note: Usually, yes, it is only called within Mobject.__init__,
+        # but there is no strong guarantee of that, and you may want certain
+        # update functions to regenerate points here and there.
         for submob in self.submobjects:
             self.remove(submob)
 
@@ -312,7 +328,7 @@ class AmbientLight(VMobject):
 
 
     def move_source_to(self,point):
-
+        # Note: Best to rewrite in terms of VectorizedPoint source_point
         v = np.array(point) - self.source_point
         self.source_point = np.array(point)
         self.shift(v)
@@ -347,6 +363,10 @@ class Spotlight(VMobject):
     }
 
     def projection_direction(self):
+        # Note: This seems reasonable, though for it to work you'd
+        # need to be sure that any 3d scene including a spotlight 
+        # somewhere assigns that spotlights "camera" attribute 
+        # to be the camera associated with that scene.
         if self.camera == None:
             return OUT
         else:
@@ -375,6 +395,10 @@ class Spotlight(VMobject):
 
 
     def new_sector(self,r,dr,lower_angle,upper_angle):
+        # Note: I'm not looking _too_ closely at the implementation
+        # of these updates based on viewing angles and such. It seems to
+        # behave as intended, but let me know if you'd like more thorough
+        # scrutiny
 
         alpha = self.max_opacity * self.opacity_function(r)
         annular_sector = AnnularSector(
@@ -466,13 +490,12 @@ class Spotlight(VMobject):
 
 
 
-
-
-
     def dimming(self,new_alpha):
         old_alpha = self.max_opacity
         self.max_opacity = new_alpha
         for submob in self.submobjects:
+            # Note: Maybe it'd be best to have a Shadow class so that the
+            # type can be checked directly?
             if type(submob) != AnnularSector:
                 # it's the shadow, don't dim it
                 continue
@@ -497,6 +520,14 @@ class Spotlight(VMobject):
             submob.set_fill(opacity = alpha)
 
 
+# Note: Stylistically, I typically keep all of the implementation for an
+# update inside the relevant Animation or ContinualAnimation, rather than
+# in the mobject.  Things are fine the way you've done it, but sometimes
+# I personally like a nice conceptual divide between all the things that 
+# determine how the mobject is displayed in a single moment (implement in
+# the mobject's class itself) and all the things determining how that changes.
+# 
+# Up to you though.
 
 class ScreenTracker(ContinualAnimation):
 

topics/three_dimensions.py

@@ -83,22 +83,22 @@ class ThreeDCamera(CameraWithPerspective):
             *self.get_spherical_coords()
         )
         def z_cmp(*vmobs):
-            #Compare to three dimensional mobjects based on 
+            # Compare to three dimensional mobjects based on 
-            #how close they are to the camera
+            # how close they are to the camera
-            return cmp(*[
-                -np.linalg.norm(vm.get_center()-camera_point)
-                for vm in vmobs
-            ])
-            # three_d_status = map(should_shade_in_3d, vmobs)
-            # has_points = [vm.get_num_points() > 0 for vm in vmobs]
-            # if all(three_d_status) and all(has_points):
-            #     cmp_vect = self.get_unit_normal_vect(vmobs[1])
             # return cmp(*[
-            #         np.dot(vm.get_center(), cmp_vect)
+            #     -np.linalg.norm(vm.get_center()-camera_point)
             #     for vm in vmobs
             # ])
-            # else:
+            three_d_status = map(should_shade_in_3d, vmobs)
-            #     return 0
+            has_points = [vm.get_num_points() > 0 for vm in vmobs]
+            if all(three_d_status) and all(has_points):
+                cmp_vect = self.get_unit_normal_vect(vmobs[1])
+                return cmp(*[
+                    np.dot(vm.get_center(), cmp_vect)
+                    for vm in vmobs
+                ])
+            else:
+                return 0
         Camera.display_multiple_vectorized_mobjects(
             self, sorted(vmobjects, cmp = z_cmp)
         )