Skip to content

Commit

Permalink
Comments cleanup
Browse files Browse the repository at this point in the history
Remove useless, outdated, mislieading or weird 'TODO'.
  • Loading branch information
TimurP committed Nov 4, 2016
1 parent 7eebe98 commit 63ca8a8
Show file tree
Hide file tree
Showing 12 changed files with 10 additions and 94 deletions.
2 changes: 0 additions & 2 deletions graf2d/cocoa/inc/QuartzPixmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@
// //
/////////////////////////////////////////////////////////

//TODO: split image and mask image?

@interface QuartzImage : NSObject<X11Drawable> {
@private
//32-bit Obj-C requires i-var to be declared (for a synthesized prop.).
Expand Down
2 changes: 0 additions & 2 deletions graf2d/cocoa/inc/TGOSXGL.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
//at some point we had to use OpenGL in our TCanvas/TPad classes which do not
//have direct access to low-level APIs + on Windows we had quite tricky
//mt-problems to deal with.
//TODO: in principle, we can get rid of gl-managers and work with TGLWidget,
//as it was demonstrated in glpad dev-branch 5 years ago.
//

class TGOSXGLManager : public TGLManager {
Expand Down
1 change: 0 additions & 1 deletion graf2d/cocoa/inc/X11Events.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class EventTranslator {
void GenerateCrossingEvent(NSEvent *theEvent);
void GeneratePointerMotionEvent(NSEvent *theEvent);

//TODO: instead of passing EMouseButton, use info from NSEvent???
void GenerateButtonPressEvent(NSView<X11Window> *eventView, NSEvent *theEvent, EMouseButton btn);
void GenerateButtonReleaseEvent(NSView<X11Window> *eventView, NSEvent *theEvent, EMouseButton btn);

Expand Down
2 changes: 1 addition & 1 deletion graf2d/cocoa/src/FontCache.mm
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ void CreateXLFDString(const X11::XLFDName &xlfd, std::string &xlfdString)
} else
xlfdString += "-*";

xlfdString += "-*-*-*-*-*-*-*-";//TODO: something more reasonable?
xlfdString += "-*-*-*-*-*-*-*-";
}

}
Expand Down
10 changes: 0 additions & 10 deletions graf2d/cocoa/src/QuartzPixmap.mm
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ @implementation QuartzPixmap

@synthesize fID;

// TODO: std::vector can be an i-var in Objective-C++,
// this will simplify and clear the error handling and
// memory management: fData does not have to be a raw pointer.

//______________________________________________________________________________
- (id) initWithW : (unsigned) width H : (unsigned) height scaleFactor : (CGFloat) scaleFactor
{
Expand Down Expand Up @@ -99,7 +95,6 @@ - (BOOL) resizeW : (unsigned) width H : (unsigned) height scaleFactor : (CGFloat
return NO;
}

//TODO: device RGB? should it be generic?
const Util::CFScopeGuard<CGColorSpaceRef> colorSpace(CGColorSpaceCreateDeviceRGB());//[1]
if (!colorSpace.Get()) {
NSLog(@"QuartzPixmap: -resizeW:H:, CGColorSpaceCreateDeviceRGB failed");
Expand All @@ -119,7 +114,6 @@ - (BOOL) resizeW : (unsigned) width H : (unsigned) height scaleFactor : (CGFloat
if (fScaleFactor > 1)
CGContextScaleCTM(ctx.Get(), fScaleFactor, fScaleFactor);

// TODO: something like move would be better.
fContext.Reset(ctx.Release());


Expand All @@ -144,7 +138,6 @@ - (CGImageRef) createImageFromPixmap : (X11::Rectangle) cropArea

//This function is incorrect in a general case, it does not care about
//cropArea.fX and cropArea.fY, very sloppy implementation.
//TODO: either fix it or remove completely.

assert(cropArea.fX >= 0 && "createImageFromPixmap:, cropArea.fX is negative");
assert(cropArea.fY >= 0 && "createImageFromPixmap:, cropArea.fY is negative");
Expand Down Expand Up @@ -448,9 +441,6 @@ @implementation QuartzImage
@synthesize fIsStippleMask;
@synthesize fID;

//TODO: all these "ctors" were added at different times, not from the beginnning.
//Refactor them to reduce code duplication, where possible.

//______________________________________________________________________________
- (id) initWithW : (unsigned) width H : (unsigned) height data : (unsigned char *) data
{
Expand Down
26 changes: 2 additions & 24 deletions graf2d/cocoa/src/QuartzWindow.mm
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,6 @@ bool ScreenPointIsInView(NSView<X11Window> *view, Int_t x, Int_t y)
//______________________________________________________________________________
NSView<X11Window> *FindViewUnderPointer()
{
//TODO: call FindViewInPoint using cursor screen coordiantes.
const Util::AutoreleasePool pool;

if (QuartzWindow *topLevel = FindWindowUnderPointer()) {
Expand Down Expand Up @@ -528,12 +527,10 @@ bool ScreenPointIsInView(NSView<X11Window> *view, Int_t x, Int_t y)
try {
result.resize(w * h * 4);
} catch (const std::bad_alloc &) {
//TODO: check that 'resize' has no side effects in case of exception.
NSLog(@"DownscaledImageData, memory allocation failed");
return result;
}

//TODO: device RGB? should it be generic?
const Util::CFScopeGuard<CGColorSpaceRef> colorSpace(CGColorSpaceCreateDeviceRGB());//[1]
if (!colorSpace.Get()) {
NSLog(@"DownscaledImageData, CGColorSpaceCreateDeviceRGB failed");
Expand Down Expand Up @@ -649,11 +646,7 @@ void SetWindowAttributes(const SetWindowAttributes_t *attr, NSObject<X11Window>
if (mask & kWAWinGravity)
window.fWinGravity = attr->fWinGravity;

//TODO: More attributes to set -
//cursor for example, etc.
if (mask & kWAOverrideRedirect) {
//This is quite a special case.
//TODO: Must be checked yet, if I understand this correctly!
if ([(NSObject *)window isKindOfClass : [QuartzWindow class]]) {
QuartzWindow * const qw = (QuartzWindow *)window;
[qw setStyleMask : Details::kBorderlessWindowMask];
Expand Down Expand Up @@ -898,10 +891,6 @@ NSPoint GetCursorHotStop(NSImage *image, ECursor cursor)
//QuartzView -drawRect/TGCocoa. So I need a trick to identify
//this special window.

//TODO: possibly refactor these functions in a more generic way - not
//to have two separate versions for text and html.


#pragma mark - Workarounds for a text view and its descendants.

//______________________________________________________________________________
Expand Down Expand Up @@ -1147,8 +1136,6 @@ - (id) initWithContentRect : (NSRect) contentRect styleMask : (NSUInteger) windo
contentViewRect.origin.x = 0.f;
contentViewRect.origin.y = 0.f;

//TODO: OpenGL view can not be content of our QuartzWindow, check if
//this is a problem for ROOT.
fContentView = [[QuartzView alloc] initWithFrame : contentViewRect windowAttributes : 0];

[self setContentView : fContentView];
Expand Down Expand Up @@ -1314,7 +1301,6 @@ - (void) setFShapeCombineMask : (QuartzImage *) mask
[fShapeCombineMask release];
if (mask) {
fShapeCombineMask = [mask retain];

//TODO: Check window's shadow???
}
}
Expand Down Expand Up @@ -1651,11 +1637,6 @@ - (BOOL) windowShouldClose : (id) sender
if (!fContentView)
return NO;

//TODO: check this!!! Children are
//transient windows and ROOT does not handle
//such a deletion properly, noop then:
//you can not close some window, if there is a
//modal dialog above.
if ([[self childWindows] count])
return NO;

Expand Down Expand Up @@ -2088,8 +2069,6 @@ - (void) copyView : (QuartzView *) srcView area : (X11::Rectangle) area toPoint
//To copy one "window" to another "window", I have to ask source QuartzView to draw intself into
//bitmap, and copy this bitmap into the destination view.

//TODO: this code must be tested, with all possible cases.

assert(srcView != nil && "-copyView:area:toPoint:, parameter 'srcView' is nil");

const NSRect frame = [srcView frame];
Expand Down Expand Up @@ -2290,7 +2269,7 @@ - (unsigned char *) readColorBits : (X11::Rectangle) area
self.fContext = ctx; //Restore old context.
//
const NSInteger bitsPerPixel = [imageRep bitsPerPixel];
//TODO: ohhh :(((

assert(bitsPerPixel == 32 && "-readColorBits:, no alpha channel???");
const NSInteger bytesPerRow = [imageRep bytesPerRow];
unsigned dataWidth = bytesPerRow / (bitsPerPixel / 8);//assume an octet :(
Expand Down Expand Up @@ -2594,7 +2573,6 @@ - (void) lowerWindow
if (sibling == self)
continue;

//TODO: equal test is not good :) I have a baaad feeling about this ;)
if (NSEqualRects(sibling.frame, self.frame)) {
[sibling setOverlapped : NO];
//
Expand Down Expand Up @@ -2947,7 +2925,7 @@ - (void) mouseMoved : (NSEvent *) theEvent
assert(fID != 0 && "-mouseMoved:, fID is 0");

if (fParentView)//Suppress events in all views, except the top-level one.
return; //TODO: check, that it does not create additional problems.
return;

assert(dynamic_cast<TGCocoa *>(gVirtualX) != 0 &&
"-mouseMoved:, gVirtualX is null or not of TGCocoa type");
Expand Down
25 changes: 2 additions & 23 deletions graf2d/cocoa/src/TGCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1503,19 +1503,14 @@ void FixAscii(std::vector<UniChar> &text)
assert([fPimpl->GetDrawable(pixmapID) isKindOfClass : [QuartzImage class]] &&
"ShapeCombineMask, pixmapID parameter must point to QuartzImage object");

//TODO: nonrectangular window can be only NSWindow object,
//not NSView (and mask is attached to a window).
//This means, if some nonrectangular window is created as a child
//first, and detached later (becoming top-level), the shape will be lost.
//Find a better way to fix it.
if (fPimpl->GetWindow(windowID).fContentView.fParentView)
return;

QuartzImage * const srcImage = (QuartzImage *)fPimpl->GetDrawable(pixmapID);
assert(srcImage.fIsStippleMask == YES && "ShapeCombineMask, source image is not a stipple mask");

//TODO: there is some kind of problems with shape masks and
//flipped views, I have to do an image flip here - check this!
// There is some kind of problems with shape masks and
// flipped views, I have to do an image flip here.
const Util::NSScopeGuard<QuartzImage> image([[QuartzImage alloc] initFromImageFlipped : srcImage]);
if (image.Get()) {
QuartzWindow * const qw = fPimpl->GetWindow(windowID).fQuartzWindow;
Expand Down Expand Up @@ -1666,7 +1661,6 @@ void FixAscii(std::vector<UniChar> &text)
CGContextTranslateCTM(ctx, 0.5, 0.5);
else {
//Pixmap uses native Cocoa's left-low-corner system.
//TODO: check the line on the edge.
y1 = Int_t(X11::LocalYROOTToCocoa(drawable, y1));
y2 = Int_t(X11::LocalYROOTToCocoa(drawable, y2));
}
Expand Down Expand Up @@ -1794,7 +1788,6 @@ void FixAscii(std::vector<UniChar> &text)
h -= 1;
}
} else {
//TODO: check the line on the edge.
//Pixmap has native Cocoa's low-left-corner system.
y = Int_t(X11::LocalYROOTToCocoa(drawable, y + h));
}
Expand Down Expand Up @@ -1873,7 +1866,6 @@ void FixAscii(std::vector<UniChar> &text)

if (drawable.fIsPixmap) {
//Pixmap has low-left-corner based system.
//TODO: check how pixmap works with pattern fill.
y = Int_t(X11::LocalYROOTToCocoa(drawable, y + h));
}

Expand Down Expand Up @@ -2345,7 +2337,6 @@ void FixAscii(std::vector<UniChar> &text)
newSize.width = w;
newSize.height = h;

//TODO: what about multi-head setup and scaling factor?
Util::NSScopeGuard<QuartzPixmap> pixmap([[QuartzPixmap alloc] initWithW : w H : h
scaleFactor : [[NSScreen mainScreen] backingScaleFactor]]);
if (pixmap.Get()) {
Expand All @@ -2370,7 +2361,6 @@ void FixAscii(std::vector<UniChar> &text)
if (w == pixmap.fWidth && h == pixmap.fHeight)
return 1;

//TODO: what about multi-head setup and scale factor?
if ([pixmap resizeW : w H : h scaleFactor : [[NSScreen mainScreen] backingScaleFactor]])
return 1;

Expand Down Expand Up @@ -3206,7 +3196,6 @@ void FixAscii(std::vector<UniChar> &text)
//Scaling factor to let our OpenGL code know, that we probably
//work on a retina display.

//TODO: what about multi-head setup?
return [[NSScreen mainScreen] backingScaleFactor];
}

Expand Down Expand Up @@ -3351,8 +3340,6 @@ void FixAscii(std::vector<UniChar> &text)
//Funny enough, but if you have invisible window with visible view,
//this trick works.

//TODO: this code is a total mess, refactor.

NSView *fakeView = nil;
QuartzWindow *fakeWindow = fPimpl->GetFakeGLWindow();

Expand Down Expand Up @@ -3490,7 +3477,6 @@ void FixAscii(std::vector<UniChar> &text)
return;
}

//TODO: what about multi-head setup?
Util::NSScopeGuard<QuartzPixmap> pixmap([[QuartzPixmap alloc] initWithW : currW
H : currH scaleFactor : [[NSScreen mainScreen] backingScaleFactor]]);
if (pixmap.Get())
Expand Down Expand Up @@ -3576,8 +3562,6 @@ void FixAscii(std::vector<UniChar> &text)
Atom_t TGCocoa::InternAtom(const char *name, Bool_t onlyIfExist)
{
//X11 properties emulation.
//TODO: this is a temporary hack to make
//client message (close window) work.

assert(name != 0 && "InternAtom, parameter 'name' is null");
return FindAtom(name, !onlyIfExist);
Expand All @@ -3596,7 +3580,6 @@ void FixAscii(std::vector<UniChar> &text)
if (!windowID)//From TGWin32.
return;

//TODO: check, if this really happens and probably remove assert.
assert(!fPimpl->IsRootWindow(windowID) &&
"SetPrimarySelectionOwner, windowID parameter is a 'root' window");
assert(fPimpl->GetDrawable(windowID).fIsPixmap == NO &&
Expand Down Expand Up @@ -3728,8 +3711,6 @@ void FixAscii(std::vector<UniChar> &text)
// actually returned.
//End of comment.

//TODO: actually, property can be set for root window.
//I have to save this data somehow.
if (fPimpl->IsRootWindow(windowID))
return 0;

Expand Down Expand Up @@ -3907,7 +3888,6 @@ void FixAscii(std::vector<UniChar> &text)
return;

//Strange signature - why propertyID is a reference?
//TODO: check, if ROOT sets/deletes properties on a 'root' window.
assert(!fPimpl->IsRootWindow(windowID) &&
"DeleteProperty, parameter 'windowID' is root window");
assert(fPimpl->GetDrawable(windowID).fIsPixmap == NO &&
Expand Down Expand Up @@ -3957,7 +3937,6 @@ void FixAscii(std::vector<UniChar> &text)
//While calling XChangeProperty, it passes the address of this typelist
//and format is ... 32. I have to pack data into unsigned and force the size:
assert(sizeof(unsigned) == 4 && "SetDNDAware, sizeof(unsigned) must be 4");
//TODO: find fixed-width integer type (I do not have cstdint header at the moment?)

std::vector<unsigned> propertyData;
propertyData.push_back(4);//This '4' is from TGX11 (is it XA_ATOM???)
Expand Down
5 changes: 0 additions & 5 deletions graf2d/cocoa/src/TGOSXGL.mm
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
assert(gGLManager == 0 && "TGOSXGLManager, gGLManager is initialized");
gGLManager = this;

//TODO: do we really need this?
if (gROOT && gROOT->GetListOfSpecials())
gROOT->GetListOfSpecials()->Add(this);
}
Expand All @@ -38,7 +37,6 @@
{
//Destructor.

//TODO: do we really need this and does ROOT ever deletes 'this'?
if (gROOT && gROOT->GetListOfSpecials())
gROOT->GetListOfSpecials()->Remove(this);
}
Expand All @@ -51,9 +49,6 @@

std::vector<component_type> format;//Where is the hummer when you need one??? (I mean C++11 initializers '{xxx}').

//TODO: this values actually are quite random, as it was in TX11GLManager/TGWin32GLManager,
//find something better!

format.push_back(component_type(Rgl::kDoubleBuffer, 1));//1 means nothing, kDoubleBuffer is enough :)
format.push_back(component_type(Rgl::kStencil, 8));
format.push_back(component_type(Rgl::kDepth, 32));
Expand Down
3 changes: 1 addition & 2 deletions graf2d/cocoa/src/TGQuartz.mm
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ void ConvertPointsROOTToCocoa(Int_t nPoints, const TPoint *xy, std::vector<TPoin
return;
}

//TODO: this is copy & paste from TGX11TTF, needs more checks (indices).
// This is copy & paste from TGX11TTF:
const Int_t xo = x1 < 0 ? -x1 : 0;
const Int_t yo = y1 < 0 ? -y1 : 0;

Expand Down Expand Up @@ -991,7 +991,6 @@ void ConvertPointsROOTToCocoa(Int_t nPoints, const TPoint *xy, std::vector<TPoin
if (gEnv) {
const TString value(TString(gEnv->GetValue("Cocoa.EnableAntiAliasing", "auto")).Strip());
if (value == "auto") {
//TODO: what about multi-head setup?
[[NSScreen mainScreen] backingScaleFactor] > 1. ? fUseAA = true : fUseAA = false;
} else if (value == "no")
fUseAA = false;
Expand Down
3 changes: 1 addition & 2 deletions graf2d/cocoa/src/X11Buffer.mm
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,6 @@

//I assume here, that all XOR ops in one iteration (one Update call) must
//be for the same window (if not, there is no normal way to implement this at all).
//TODO: verify and check this condition.

NSObject<X11Drawable> *drawable = impl->GetDrawable(fXorOps[0]->fID);

Expand Down Expand Up @@ -912,7 +911,7 @@ int_iterator BinarySearchRight(int_iterator first, int_iterator last, int value)

//This is quite straightforward implementation - I'm calculation rectangles, which are part of
//a widget's rect, not hidden by any of fRectsToClip.
//TODO: find a better algorithm.

typedef std::vector<WidgetRect>::const_iterator rect_const_iterator;
typedef std::vector<bool>::size_type size_type;

Expand Down
Loading

0 comments on commit 63ca8a8

Please sign in to comment.